* [alsa-devel] RFC: TDM mode support on the tegra30
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel
This is an initial set that adds support for both the TDM and for
formats other than 8/16bit audio through the audio system. It also
fixes a minor issue with the FIFO stopping order.
I have further patches following on which deal with selection of
clocks to allow the use of the tegra's audio unit to run as clock
slave (all current examples have the tegra as clock master).
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
driver.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index ac6983c6bd72..d36b4662b420 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
return 0;
}
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+ unsigned int tx_mask, unsigned int rx_mask,
+ int slots, int slot_width)
+{
+ struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ unsigned int mask = 0, val = 0;
+
+ dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
+ "slots: 0x%08x " "width: %d\n",
+ __func__, tx_mask, rx_mask, slots, slot_width);
+
+ /* Set up slots and tx/rx masks */
+ mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
+ TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+ TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+ val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+ (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+ ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+
+ /* Set FSYNC width */
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
+ TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
+ (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
+
+ return 0;
+}
+
static int tegra30_i2s_probe(struct snd_soc_dai *dai)
{
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -268,6 +301,7 @@ static const struct snd_soc_dai_ops tegra30_i2s_dai_ops = {
.set_fmt = tegra30_i2s_set_fmt,
.hw_params = tegra30_i2s_hw_params,
.trigger = tegra30_i2s_trigger,
+ .set_tdm_slot = tegra30_i2s_set_tdm,
};
static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread* [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
driver.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index ac6983c6bd72..d36b4662b420 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
return 0;
}
+/*
+ * Set up TDM
+ */
+static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
+ unsigned int tx_mask, unsigned int rx_mask,
+ int slots, int slot_width)
+{
+ struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+ unsigned int mask = 0, val = 0;
+
+ dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
+ "slots: 0x%08x " "width: %d\n",
+ __func__, tx_mask, rx_mask, slots, slot_width);
+
+ /* Set up slots and tx/rx masks */
+ mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
+ TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
+ TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
+
+ val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
+ (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
+ ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
+
+ /* Set FSYNC width */
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
+ TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
+ (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
+
+ return 0;
+}
+
static int tegra30_i2s_probe(struct snd_soc_dai *dai)
{
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
@@ -268,6 +301,7 @@ static const struct snd_soc_dai_ops tegra30_i2s_dai_ops = {
.set_fmt = tegra30_i2s_set_fmt,
.hw_params = tegra30_i2s_hw_params,
.trigger = tegra30_i2s_trigger,
+ .set_tdm_slot = tegra30_i2s_set_tdm,
};
static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:22 ` Pierre-Louis Bossart
-1 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-17 18:22 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding,
Jonathan Hunter
Cc: linux-kernel, Edward Cragg
On 9/17/19 1:12 PM, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
> driver.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index ac6983c6bd72..d36b4662b420 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> return 0;
> }
>
> +/*
> + * Set up TDM
> + */
> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> + unsigned int tx_mask, unsigned int rx_mask,
> + int slots, int slot_width)
> +{
> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + unsigned int mask = 0, val = 0;
initialization is not needed.
> +
> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
> + "slots: 0x%08x " "width: %d\n",
> + __func__, tx_mask, rx_mask, slots, slot_width);
> +
> + /* Set up slots and tx/rx masks */
> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
> +
> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
> +
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
> +
> + /* Set FSYNC width */
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
> +
> + return 0;
> +}
> +
> static int tegra30_i2s_probe(struct snd_soc_dai *dai)
> {
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> @@ -268,6 +301,7 @@ static const struct snd_soc_dai_ops tegra30_i2s_dai_ops = {
> .set_fmt = tegra30_i2s_set_fmt,
> .hw_params = tegra30_i2s_hw_params,
> .trigger = tegra30_i2s_trigger,
> + .set_tdm_slot = tegra30_i2s_set_tdm,
> };
>
> static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-17 18:22 ` Pierre-Louis Bossart
0 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-17 18:22 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding,
Jonathan Hunter
Cc: linux-kernel, Edward Cragg
On 9/17/19 1:12 PM, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
> driver.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index ac6983c6bd72..d36b4662b420 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> return 0;
> }
>
> +/*
> + * Set up TDM
> + */
> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> + unsigned int tx_mask, unsigned int rx_mask,
> + int slots, int slot_width)
> +{
> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + unsigned int mask = 0, val = 0;
initialization is not needed.
> +
> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
> + "slots: 0x%08x " "width: %d\n",
> + __func__, tx_mask, rx_mask, slots, slot_width);
> +
> + /* Set up slots and tx/rx masks */
> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
> +
> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
> +
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
> +
> + /* Set FSYNC width */
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
> +
> + return 0;
> +}
> +
> static int tegra30_i2s_probe(struct snd_soc_dai *dai)
> {
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> @@ -268,6 +301,7 @@ static const struct snd_soc_dai_ops tegra30_i2s_dai_ops = {
> .set_fmt = tegra30_i2s_set_fmt,
> .hw_params = tegra30_i2s_hw_params,
> .trigger = tegra30_i2s_trigger,
> + .set_tdm_slot = tegra30_i2s_set_tdm,
> };
>
> static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 8:42 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:42 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Edward Cragg
On 17/09/2019 19:12, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
> driver.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index ac6983c6bd72..d36b4662b420 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> return 0;
> }
>
> +/*
> + * Set up TDM
> + */
> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> + unsigned int tx_mask, unsigned int rx_mask,
> + int slots, int slot_width)
> +{
> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + unsigned int mask = 0, val = 0;
> +
> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
dev_dbg() please. Also I don't think it is necessary to print both the
function name and 'setting TDM', the function name should be sufficient.
> + "slots: 0x%08x " "width: %d\n",
Why are there extra quotes here?
> + __func__, tx_mask, rx_mask, slots, slot_width)> +
> + /* Set up slots and tx/rx masks */
> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
> +
> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
> +
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
> +
> + /* Set FSYNC width */
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
What happens if there is only one slot? The fsync will be the width of
the slot. Typically, TDM is used with DSP-A/B formats and although the
driver does not appear to program the fsync width, it probably should
during the tegra30_i2s_set_fmt() depending on the format used rather
than here.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 8:42 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:42 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Edward Cragg
On 17/09/2019 19:12, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> Add a callback to configure TDM settings for the Tegra30 I2S ASoC 'platform'
> driver.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index ac6983c6bd72..d36b4662b420 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
> return 0;
> }
>
> +/*
> + * Set up TDM
> + */
> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> + unsigned int tx_mask, unsigned int rx_mask,
> + int slots, int slot_width)
> +{
> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> + unsigned int mask = 0, val = 0;
> +
> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
dev_dbg() please. Also I don't think it is necessary to print both the
function name and 'setting TDM', the function name should be sufficient.
> + "slots: 0x%08x " "width: %d\n",
Why are there extra quotes here?
> + __func__, tx_mask, rx_mask, slots, slot_width)> +
> + /* Set up slots and tx/rx masks */
> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
> +
> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
> +
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
> +
> + /* Set FSYNC width */
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
What happens if there is only one slot? The fsync will be the width of
the slot. Typically, TDM is used with DSP-A/B formats and although the
driver does not appear to program the fsync width, it probably should
during the tegra30_i2s_set_fmt() depending on the format used rather
than here.
Cheers
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 8:42 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 10:11 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:11 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 2019-09-18 09:42, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> Add a callback to configure TDM settings for the Tegra30 I2S ASoC
>> 'platform'
>> driver.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index ac6983c6bd72..d36b4662b420 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct
>> snd_pcm_substream *substream, int cmd,
>> return 0;
>> }
>>
>> +/*
>> + * Set up TDM
>> + */
>> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
>> + unsigned int tx_mask, unsigned int rx_mask,
>> + int slots, int slot_width)
>> +{
>> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> + unsigned int mask = 0, val = 0;
>> +
>> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x
>> "
>
> dev_dbg() please. Also I don't think it is necessary to print both the
> function name and 'setting TDM', the function name should be
> sufficient.
Yes, already sorted from previous review.
>> + "slots: 0x%08x " "width: %d\n",
>
> Why are there extra quotes here?
No idea, I'll remove these later.
>> + __func__, tx_mask, rx_mask, slots, slot_width)> +
>> + /* Set up slots and tx/rx masks */
>> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
>> +
>> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
>> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>> +
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>> +
>> + /* Set FSYNC width */
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
>> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>
> What happens if there is only one slot? The fsync will be the width of
> the slot. Typically, TDM is used with DSP-A/B formats and although the
> driver does not appear to program the fsync width, it probably should
> during the tegra30_i2s_set_fmt() depending on the format used rather
> than here.
Hmm, should we check.
The work was done to keep as close to the original client's 2.6 kernel
as possible which set the fsync field to a whole data word. We could try
and just set this to say 2 here and have a much shorter frame-sync
pulse.
I'll add a check for slots < 2 and set the fsync width to 2.
Thanks for the review.
>
> Cheers
> Jon
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 10:11 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:11 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 2019-09-18 09:42, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> Add a callback to configure TDM settings for the Tegra30 I2S ASoC
>> 'platform'
>> driver.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index ac6983c6bd72..d36b4662b420 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct
>> snd_pcm_substream *substream, int cmd,
>> return 0;
>> }
>>
>> +/*
>> + * Set up TDM
>> + */
>> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
>> + unsigned int tx_mask, unsigned int rx_mask,
>> + int slots, int slot_width)
>> +{
>> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> + unsigned int mask = 0, val = 0;
>> +
>> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x
>> "
>
> dev_dbg() please. Also I don't think it is necessary to print both the
> function name and 'setting TDM', the function name should be
> sufficient.
Yes, already sorted from previous review.
>> + "slots: 0x%08x " "width: %d\n",
>
> Why are there extra quotes here?
No idea, I'll remove these later.
>> + __func__, tx_mask, rx_mask, slots, slot_width)> +
>> + /* Set up slots and tx/rx masks */
>> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
>> +
>> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
>> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>> +
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>> +
>> + /* Set FSYNC width */
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
>> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>
> What happens if there is only one slot? The fsync will be the width of
> the slot. Typically, TDM is used with DSP-A/B formats and although the
> driver does not appear to program the fsync width, it probably should
> during the tegra30_i2s_set_fmt() depending on the format used rather
> than here.
Hmm, should we check.
The work was done to keep as close to the original client's 2.6 kernel
as possible which set the fsync field to a whole data word. We could try
and just set this to say 2 here and have a much shorter frame-sync
pulse.
I'll add a check for slots < 2 and set the fsync width to 2.
Thanks for the review.
>
> Cheers
> Jon
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 10:11 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 10:25 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 10:25 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 18/09/2019 11:11, Ben Dooks wrote:
>
>
> On 2019-09-18 09:42, Jon Hunter wrote:
>> On 17/09/2019 19:12, Ben Dooks wrote:
>>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>>
>>> Add a callback to configure TDM settings for the Tegra30 I2S ASoC
>>> 'platform'
>>> driver.
>>>
>>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>>> ---
>>> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>> b/sound/soc/tegra/tegra30_i2s.c
>>> index ac6983c6bd72..d36b4662b420 100644
>>> --- a/sound/soc/tegra/tegra30_i2s.c
>>> +++ b/sound/soc/tegra/tegra30_i2s.c
>>> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct
>>> snd_pcm_substream *substream, int cmd,
>>> return 0;
>>> }
>>>
>>> +/*
>>> + * Set up TDM
>>> + */
>>> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
>>> + unsigned int tx_mask, unsigned int rx_mask,
>>> + int slots, int slot_width)
>>> +{
>>> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> + unsigned int mask = 0, val = 0;
>>> +
>>> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask:
>>> 0x%08x "
>>
>> dev_dbg() please. Also I don't think it is necessary to print both the
>> function name and 'setting TDM', the function name should be sufficient.
>
> Yes, already sorted from previous review.
>
>>> + "slots: 0x%08x " "width: %d\n",
>>
>> Why are there extra quotes here?
>
> No idea, I'll remove these later.
>
>>> + __func__, tx_mask, rx_mask, slots, slot_width)> +
>>> + /* Set up slots and tx/rx masks */
>>> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>>> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>>> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
>>> +
>>> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
>>> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>>> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>>> +
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>>> +
>>> + /* Set FSYNC width */
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
>>> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>>> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>>
>> What happens if there is only one slot? The fsync will be the width of
>> the slot. Typically, TDM is used with DSP-A/B formats and although the
>> driver does not appear to program the fsync width, it probably should
>> during the tegra30_i2s_set_fmt() depending on the format used rather
>> than here.
>
> Hmm, should we check.
>
> The work was done to keep as close to the original client's 2.6 kernel
> as possible which set the fsync field to a whole data word. We could try
> and just set this to say 2 here and have a much shorter frame-sync pulse.
>
> I'll add a check for slots < 2 and set the fsync width to 2.
Why 2? From looking at various codecs that support dsp-a/b modes, it is
more common for the f-sync to be 1 regardless of the number of slots.
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 10:25 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 10:25 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 18/09/2019 11:11, Ben Dooks wrote:
>
>
> On 2019-09-18 09:42, Jon Hunter wrote:
>> On 17/09/2019 19:12, Ben Dooks wrote:
>>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>>
>>> Add a callback to configure TDM settings for the Tegra30 I2S ASoC
>>> 'platform'
>>> driver.
>>>
>>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>>> ---
>>> sound/soc/tegra/tegra30_i2s.c | 34 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>> b/sound/soc/tegra/tegra30_i2s.c
>>> index ac6983c6bd72..d36b4662b420 100644
>>> --- a/sound/soc/tegra/tegra30_i2s.c
>>> +++ b/sound/soc/tegra/tegra30_i2s.c
>>> @@ -254,6 +254,39 @@ static int tegra30_i2s_trigger(struct
>>> snd_pcm_substream *substream, int cmd,
>>> return 0;
>>> }
>>>
>>> +/*
>>> + * Set up TDM
>>> + */
>>> +static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
>>> + unsigned int tx_mask, unsigned int rx_mask,
>>> + int slots, int slot_width)
>>> +{
>>> + struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>>> + unsigned int mask = 0, val = 0;
>>> +
>>> + dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask:
>>> 0x%08x "
>>
>> dev_dbg() please. Also I don't think it is necessary to print both the
>> function name and 'setting TDM', the function name should be sufficient.
>
> Yes, already sorted from previous review.
>
>>> + "slots: 0x%08x " "width: %d\n",
>>
>> Why are there extra quotes here?
>
> No idea, I'll remove these later.
>
>>> + __func__, tx_mask, rx_mask, slots, slot_width)> +
>>> + /* Set up slots and tx/rx masks */
>>> + mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>>> + TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>>> + TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_MASK;
>>> +
>>> + val = (tx_mask << TEGRA30_I2S_SLOT_CTRL_TX_SLOT_ENABLES_SHIFT) |
>>> + (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>>> + ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>>> +
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>>> +
>>> + /* Set FSYNC width */
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL,
>>> + TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>>> + (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>>
>> What happens if there is only one slot? The fsync will be the width of
>> the slot. Typically, TDM is used with DSP-A/B formats and although the
>> driver does not appear to program the fsync width, it probably should
>> during the tegra30_i2s_set_fmt() depending on the format used rather
>> than here.
>
> Hmm, should we check.
>
> The work was done to keep as close to the original client's 2.6 kernel
> as possible which set the fsync field to a whole data word. We could try
> and just set this to say 2 here and have a much shorter frame-sync pulse.
>
> I'll add a check for slots < 2 and set the fsync width to 2.
Why 2? From looking at various codecs that support dsp-a/b modes, it is
more common for the f-sync to be 1 regardless of the number of slots.
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 10:25 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 10:48 ` Mark Brown
-1 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 10:48 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 308 bytes --]
On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
> Why 2? From looking at various codecs that support dsp-a/b modes, it is
> more common for the f-sync to be 1 regardless of the number of slots.
In DSP modes only one edge really matters anyway so it's not super
important how long the pulse is.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 10:48 ` Mark Brown
0 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 10:48 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 308 bytes --]
On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
> Why 2? From looking at various codecs that support dsp-a/b modes, it is
> more common for the f-sync to be 1 regardless of the number of slots.
In DSP modes only one edge really matters anyway so it's not super
important how long the pulse is.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 10:48 ` [alsa-devel] " Mark Brown
@ 2019-09-18 11:44 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:44 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, alsa-devel, Takashi Iwai, Liam Girdwood,
Thierry Reding, Edward Cragg, linux-tegra, Jon Hunter,
linux-tegra-owner
On 2019-09-18 11:48, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
>
>> Why 2? From looking at various codecs that support dsp-a/b modes, it
>> is
>> more common for the f-sync to be 1 regardless of the number of slots.
>
> In DSP modes only one edge really matters anyway so it's not super
> important how long the pulse is.
I could never get an answer for why this was set as-is on the customer's
setup and not sure if I have the ability to re-test this.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 11:44 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:44 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, alsa-devel, Takashi Iwai, Liam Girdwood,
Thierry Reding, Edward Cragg, linux-tegra, Jon Hunter,
linux-tegra-owner
On 2019-09-18 11:48, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
>
>> Why 2? From looking at various codecs that support dsp-a/b modes, it
>> is
>> more common for the f-sync to be 1 regardless of the number of slots.
>
> In DSP modes only one edge really matters anyway so it's not super
> important how long the pulse is.
I could never get an answer for why this was set as-is on the customer's
setup and not sure if I have the ability to re-test this.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 10:48 ` [alsa-devel] " Mark Brown
@ 2019-09-18 13:33 ` Pierre-Louis Bossart
-1 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-18 13:33 UTC (permalink / raw)
To: Mark Brown, Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, linux-tegra-owner
On 9/18/19 5:48 AM, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
>
>> Why 2? From looking at various codecs that support dsp-a/b modes, it is
>> more common for the f-sync to be 1 regardless of the number of slots.
>
> In DSP modes only one edge really matters anyway so it's not super
> important how long the pulse is.
There are exceptions to the rule.
In the early days of SOF, we had to provide support for amplifiers that
did require a pulse larger than a bit. In the SOF IPC we added an
'frame_pulse_width' field to pass the configuration all the way from
topology to the firmware and Intel SSP driver.
The other quirk we added is the ability to control zero-padding per slot
instead of at the end of the frame, e.g. 1 bit of padding after 24 bits
when using 4 slots w/ 25 bits in a 100-bit frame.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 13:33 ` Pierre-Louis Bossart
0 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-18 13:33 UTC (permalink / raw)
To: Mark Brown, Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, linux-tegra-owner
On 9/18/19 5:48 AM, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 11:25:39AM +0100, Jon Hunter wrote:
>
>> Why 2? From looking at various codecs that support dsp-a/b modes, it is
>> more common for the f-sync to be 1 regardless of the number of slots.
>
> In DSP modes only one edge really matters anyway so it's not super
> important how long the pulse is.
There are exceptions to the rule.
In the early days of SOF, we had to provide support for amplifiers that
did require a pulse larger than a bit. In the SOF IPC we added an
'frame_pulse_width' field to pass the configuration all the way from
topology to the firmware and Intel SSP driver.
The other quirk we added is the ability to control zero-padding per slot
instead of at the end of the frame, e.g. 1 bit of padding after 24 bits
when using 4 slots w/ 25 bits in a 100-bit frame.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
2019-09-18 13:33 ` [alsa-devel] " Pierre-Louis Bossart
@ 2019-09-18 15:02 ` Mark Brown
-1 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 15:02 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: linux-kernel, alsa-devel, Takashi Iwai, Liam Girdwood, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, Jon Hunter,
linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 877 bytes --]
On Wed, Sep 18, 2019 at 08:33:50AM -0500, Pierre-Louis Bossart wrote:
> On 9/18/19 5:48 AM, Mark Brown wrote:
> > In DSP modes only one edge really matters anyway so it's not super
> > important how long the pulse is.
> There are exceptions to the rule.
> In the early days of SOF, we had to provide support for amplifiers that did
> require a pulse larger than a bit. In the SOF IPC we added an
> 'frame_pulse_width' field to pass the configuration all the way from
> topology to the firmware and Intel SSP driver.
> The other quirk we added is the ability to control zero-padding per slot
> instead of at the end of the frame, e.g. 1 bit of padding after 24 bits when
> using 4 slots w/ 25 bits in a 100-bit frame.
Neither of those is part of the core DSP mode definition though in the
same way that constraints like MCLK or BCLK ratios aren't. They're
modifiers on top.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 1/8] ASoC: tegra: Add a TDM configuration callback
@ 2019-09-18 15:02 ` Mark Brown
0 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 15:02 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: linux-kernel, alsa-devel, Takashi Iwai, Liam Girdwood, Ben Dooks,
Thierry Reding, Edward Cragg, linux-tegra, Jon Hunter,
linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 877 bytes --]
On Wed, Sep 18, 2019 at 08:33:50AM -0500, Pierre-Louis Bossart wrote:
> On 9/18/19 5:48 AM, Mark Brown wrote:
> > In DSP modes only one edge really matters anyway so it's not super
> > important how long the pulse is.
> There are exceptions to the rule.
> In the early days of SOF, we had to provide support for amplifiers that did
> require a pulse larger than a bit. In the SOF IPC we added an
> 'frame_pulse_width' field to pass the configuration all the way from
> topology to the firmware and Intel SSP driver.
> The other quirk we added is the ability to control zero-padding per slot
> instead of at the end of the frame, e.g. 1 bit of padding after 24 bits when
> using 4 slots w/ 25 bits in a 100-bit frame.
Neither of those is part of the core DSP mode definition though in the
same way that constraints like MCLK or BCLK ratios aren't. They're
modifiers on top.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
The tegra3 audio can support 24 and 32 bit sample sizes so add the
option to the tegra30_i2s_hw_params to configure the S24_LE or
S32_LE formats when requested.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
[ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
[ben.dooks@codethink.co.uk: add pm calls around ytdm config]
[ben.dooks@codethink.co.uk: remove debug prints]
---
squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
---
sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index d36b4662b420..b5372839f672 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
struct device *dev = dai->dev;
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask, val, reg;
- int ret, sample_size, srate, i2sclock, bitcnt;
+ int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
struct tegra30_ahub_cif_conf cif_conf;
if (params_channels(params) != 2)
@@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_16;
sample_size = 16;
break;
+ case SNDRV_PCM_FORMAT_S24_LE:
+ val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_24;
+ sample_size = 24;
+ break;
+ case SNDRV_PCM_FORMAT_S32_LE:
+ val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_32;
+ sample_size = 32;
+ break;
default:
return -EINVAL;
}
@@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
cif_conf.threshold = 0;
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
- cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
+ cif_conf.audio_bits = audio_bits;
+ cif_conf.client_bits = audio_bits;
cif_conf.expand = 0;
cif_conf.stereo_conv = 0;
cif_conf.replicate = 0;
@@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask = 0, val = 0;
- dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
- "slots: 0x%08x " "width: %d\n",
- __func__, tx_mask, rx_mask, slots, slot_width);
-
/* Set up slots and tx/rx masks */
mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
@@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
(rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+ pm_runtime_get_sync(dai->dev);
+
regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
/* Set FSYNC width */
@@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
(slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
+ pm_runtime_put(dai->dev);
return 0;
}
@@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_96000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE |
+ SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S16_LE,
},
.capture = {
.stream_name = "Capture",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_96000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE |
+ SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &tegra30_i2s_dai_ops,
.symmetric_rates = 1,
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread* [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
The tegra3 audio can support 24 and 32 bit sample sizes so add the
option to the tegra30_i2s_hw_params to configure the S24_LE or
S32_LE formats when requested.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
[ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
[ben.dooks@codethink.co.uk: add pm calls around ytdm config]
[ben.dooks@codethink.co.uk: remove debug prints]
---
squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
---
sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index d36b4662b420..b5372839f672 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
struct device *dev = dai->dev;
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask, val, reg;
- int ret, sample_size, srate, i2sclock, bitcnt;
+ int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
struct tegra30_ahub_cif_conf cif_conf;
if (params_channels(params) != 2)
@@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_16;
sample_size = 16;
break;
+ case SNDRV_PCM_FORMAT_S24_LE:
+ val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_24;
+ sample_size = 24;
+ break;
+ case SNDRV_PCM_FORMAT_S32_LE:
+ val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
+ audio_bits = TEGRA30_AUDIOCIF_BITS_32;
+ sample_size = 32;
+ break;
default:
return -EINVAL;
}
@@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
cif_conf.threshold = 0;
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
- cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
+ cif_conf.audio_bits = audio_bits;
+ cif_conf.client_bits = audio_bits;
cif_conf.expand = 0;
cif_conf.stereo_conv = 0;
cif_conf.replicate = 0;
@@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask = 0, val = 0;
- dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
- "slots: 0x%08x " "width: %d\n",
- __func__, tx_mask, rx_mask, slots, slot_width);
-
/* Set up slots and tx/rx masks */
mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
@@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
(rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
+ pm_runtime_get_sync(dai->dev);
+
regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
/* Set FSYNC width */
@@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
(slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
+ pm_runtime_put(dai->dev);
return 0;
}
@@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_96000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE |
+ SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S16_LE,
},
.capture = {
.stream_name = "Capture",
.channels_min = 2,
.channels_max = 2,
.rates = SNDRV_PCM_RATE_8000_96000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE |
+ SNDRV_PCM_FMTBIT_S24_LE |
+ SNDRV_PCM_FMTBIT_S16_LE,
},
.ops = &tegra30_i2s_dai_ops,
.symmetric_rates = 1,
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:26 ` Pierre-Louis Bossart
-1 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-17 18:26 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding,
Jonathan Hunter
Cc: linux-kernel, Edward Cragg
On 9/17/19 1:12 PM, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> The tegra3 audio can support 24 and 32 bit sample sizes so add the
> option to the tegra30_i2s_hw_params to configure the S24_LE or
> S32_LE formats when requested.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> [ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
> [ben.dooks@codethink.co.uk: add pm calls around ytdm config]
> [ben.dooks@codethink.co.uk: remove debug prints]
You need to add your own Signed-off-by when sending patches from other
people
> ---
> squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
>
> ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
> ---
> sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index d36b4662b420..b5372839f672 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> struct device *dev = dai->dev;
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask, val, reg;
> - int ret, sample_size, srate, i2sclock, bitcnt;
> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
> struct tegra30_ahub_cif_conf cif_conf;
>
> if (params_channels(params) != 2)
> @@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> switch (params_format(params)) {
> case SNDRV_PCM_FORMAT_S16_LE:
> val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_16;
> sample_size = 16;
> break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_24;
> + sample_size = 24;
> + break;
> + case SNDRV_PCM_FORMAT_S32_LE:
> + val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_32;
> + sample_size = 32;
> + break;
> default:
> return -EINVAL;
> }
> @@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> cif_conf.threshold = 0;
> cif_conf.audio_channels = 2;
> cif_conf.client_channels = 2;
> - cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
> - cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
> + cif_conf.audio_bits = audio_bits;
> + cif_conf.client_bits = audio_bits;
> cif_conf.expand = 0;
> cif_conf.stereo_conv = 0;
> cif_conf.replicate = 0;
> @@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask = 0, val = 0;
>
> - dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
> - "slots: 0x%08x " "width: %d\n",
> - __func__, tx_mask, rx_mask, slots, slot_width);
> -
This should be squashed in the previous patch
> /* Set up slots and tx/rx masks */
> mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> @@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>
> + pm_runtime_get_sync(dai->dev);
> +
> regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>
> /* Set FSYNC width */
> @@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>
> + pm_runtime_put(dai->dev);
same for PM stuff, it's not related to 24/32 bit samples
> return 0;
> }
>
> @@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
> .channels_min = 2,
> .channels_max = 2,
> .rates = SNDRV_PCM_RATE_8000_96000,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
> + SNDRV_PCM_FMTBIT_S24_LE |
> + SNDRV_PCM_FMTBIT_S16_LE,
> },
> .capture = {
> .stream_name = "Capture",
> .channels_min = 2,
> .channels_max = 2,
> .rates = SNDRV_PCM_RATE_8000_96000,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
> + SNDRV_PCM_FMTBIT_S24_LE |
> + SNDRV_PCM_FMTBIT_S16_LE,
> },
> .ops = &tegra30_i2s_dai_ops,
> .symmetric_rates = 1,
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-17 18:26 ` Pierre-Louis Bossart
0 siblings, 0 replies; 68+ messages in thread
From: Pierre-Louis Bossart @ 2019-09-17 18:26 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding,
Jonathan Hunter
Cc: linux-kernel, Edward Cragg
On 9/17/19 1:12 PM, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> The tegra3 audio can support 24 and 32 bit sample sizes so add the
> option to the tegra30_i2s_hw_params to configure the S24_LE or
> S32_LE formats when requested.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> [ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
> [ben.dooks@codethink.co.uk: add pm calls around ytdm config]
> [ben.dooks@codethink.co.uk: remove debug prints]
You need to add your own Signed-off-by when sending patches from other
people
> ---
> squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
>
> ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
> ---
> sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index d36b4662b420..b5372839f672 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> struct device *dev = dai->dev;
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask, val, reg;
> - int ret, sample_size, srate, i2sclock, bitcnt;
> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
> struct tegra30_ahub_cif_conf cif_conf;
>
> if (params_channels(params) != 2)
> @@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> switch (params_format(params)) {
> case SNDRV_PCM_FORMAT_S16_LE:
> val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_16;
> sample_size = 16;
> break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_24;
> + sample_size = 24;
> + break;
> + case SNDRV_PCM_FORMAT_S32_LE:
> + val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
> + audio_bits = TEGRA30_AUDIOCIF_BITS_32;
> + sample_size = 32;
> + break;
> default:
> return -EINVAL;
> }
> @@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> cif_conf.threshold = 0;
> cif_conf.audio_channels = 2;
> cif_conf.client_channels = 2;
> - cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
> - cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
> + cif_conf.audio_bits = audio_bits;
> + cif_conf.client_bits = audio_bits;
> cif_conf.expand = 0;
> cif_conf.stereo_conv = 0;
> cif_conf.replicate = 0;
> @@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask = 0, val = 0;
>
> - dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask: 0x%08x "
> - "slots: 0x%08x " "width: %d\n",
> - __func__, tx_mask, rx_mask, slots, slot_width);
> -
This should be squashed in the previous patch
> /* Set up slots and tx/rx masks */
> mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
> TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
> @@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
> ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>
> + pm_runtime_get_sync(dai->dev);
> +
> regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>
> /* Set FSYNC width */
> @@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai *dai,
> TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
> (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>
> + pm_runtime_put(dai->dev);
same for PM stuff, it's not related to 24/32 bit samples
> return 0;
> }
>
> @@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
> .channels_min = 2,
> .channels_max = 2,
> .rates = SNDRV_PCM_RATE_8000_96000,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
> + SNDRV_PCM_FMTBIT_S24_LE |
> + SNDRV_PCM_FMTBIT_S16_LE,
> },
> .capture = {
> .stream_name = "Capture",
> .channels_min = 2,
> .channels_max = 2,
> .rates = SNDRV_PCM_RATE_8000_96000,
> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
> + SNDRV_PCM_FMTBIT_S24_LE |
> + SNDRV_PCM_FMTBIT_S16_LE,
> },
> .ops = &tegra30_i2s_dai_ops,
> .symmetric_rates = 1,
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-17 18:26 ` [alsa-devel] " Pierre-Louis Bossart
@ 2019-09-18 7:44 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 7:44 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Jonathan Hunter, linux-kernel, alsa-devel, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
> On 9/17/19 1:12 PM, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> The tegra3 audio can support 24 and 32 bit sample sizes so add the
>> option to the tegra30_i2s_hw_params to configure the S24_LE or
>> S32_LE formats when requested.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> [ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
>> [ben.dooks@codethink.co.uk: add pm calls around ytdm config]
>> [ben.dooks@codethink.co.uk: remove debug prints]
>
> You need to add your own Signed-off-by when sending patches from other
> people
Yes, will do when this series has been reviewed and modifications done.
>
>> ---
>> squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed
>> in tdm code
>>
>> ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
>> 1 file changed, 23 insertions(+), 9 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index d36b4662b420..b5372839f672 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> struct device *dev = dai->dev;
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask, val, reg;
>> - int ret, sample_size, srate, i2sclock, bitcnt;
>> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
>> struct tegra30_ahub_cif_conf cif_conf;
>> if (params_channels(params) != 2)
>> @@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> switch (params_format(params)) {
>> case SNDRV_PCM_FORMAT_S16_LE:
>> val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>> sample_size = 16;
>> break;
>> + case SNDRV_PCM_FORMAT_S24_LE:
>> + val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_24;
>> + sample_size = 24;
>> + break;
>> + case SNDRV_PCM_FORMAT_S32_LE:
>> + val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_32;
>> + sample_size = 32;
>> + break;
>> default:
>> return -EINVAL;
>> }
>> @@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> cif_conf.threshold = 0;
>> cif_conf.audio_channels = 2;
>> cif_conf.client_channels = 2;
>> - cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>> - cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
>> + cif_conf.audio_bits = audio_bits;
>> + cif_conf.client_bits = audio_bits;
>> cif_conf.expand = 0;
>> cif_conf.stereo_conv = 0;
>> cif_conf.replicate = 0;
>> @@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask = 0, val = 0;
>> - dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask:
>> 0x%08x "
>> - "slots: 0x%08x " "width: %d\n",
>> - __func__, tx_mask, rx_mask, slots, slot_width);
>> -
>
> This should be squashed in the previous patch
Thanks, looks like I missed a bad rebase and hit this patch instead of
the
previous.
>
>> /* Set up slots and tx/rx masks */
>> mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>> TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>> @@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>> ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>> + pm_runtime_get_sync(dai->dev);
>> +
>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>> /* Set FSYNC width */
>> @@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>> (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>> + pm_runtime_put(dai->dev);
>
> same for PM stuff, it's not related to 24/32 bit samples
Yes, will do.
Thank you for reviewing.
>> return 0;
>> }
>> @@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver
>> tegra30_i2s_dai_template = {
>> .channels_min = 2,
>> .channels_max = 2,
>> .rates = SNDRV_PCM_RATE_8000_96000,
>> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
>> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE |
>> + SNDRV_PCM_FMTBIT_S16_LE,
>> },
>> .capture = {
>> .stream_name = "Capture",
>> .channels_min = 2,
>> .channels_max = 2,
>> .rates = SNDRV_PCM_RATE_8000_96000,
>> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
>> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE |
>> + SNDRV_PCM_FMTBIT_S16_LE,
>> },
>> .ops = &tegra30_i2s_dai_ops,
>> .symmetric_rates = 1,
>>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-18 7:44 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 7:44 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Jonathan Hunter, linux-kernel, alsa-devel, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
> On 9/17/19 1:12 PM, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> The tegra3 audio can support 24 and 32 bit sample sizes so add the
>> option to the tegra30_i2s_hw_params to configure the S24_LE or
>> S32_LE formats when requested.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> [ben.dooks@codethink.co.uk: fixup merge of 24 and 32bit]
>> [ben.dooks@codethink.co.uk: add pm calls around ytdm config]
>> [ben.dooks@codethink.co.uk: remove debug prints]
>
> You need to add your own Signed-off-by when sending patches from other
> people
Yes, will do when this series has been reviewed and modifications done.
>
>> ---
>> squash 5aeca5a055fd ASoC: tegra: i2s: pm_runtime_get_sync() is needed
>> in tdm code
>>
>> ASoC: tegra: i2s: pm_runtime_get_sync() is needed in tdm code
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 32 +++++++++++++++++++++++---------
>> 1 file changed, 23 insertions(+), 9 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index d36b4662b420..b5372839f672 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -127,7 +127,7 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> struct device *dev = dai->dev;
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask, val, reg;
>> - int ret, sample_size, srate, i2sclock, bitcnt;
>> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
>> struct tegra30_ahub_cif_conf cif_conf;
>> if (params_channels(params) != 2)
>> @@ -137,8 +137,19 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> switch (params_format(params)) {
>> case SNDRV_PCM_FORMAT_S16_LE:
>> val = TEGRA30_I2S_CTRL_BIT_SIZE_16;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>> sample_size = 16;
>> break;
>> + case SNDRV_PCM_FORMAT_S24_LE:
>> + val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_24;
>> + sample_size = 24;
>> + break;
>> + case SNDRV_PCM_FORMAT_S32_LE:
>> + val = TEGRA30_I2S_CTRL_BIT_SIZE_32;
>> + audio_bits = TEGRA30_AUDIOCIF_BITS_32;
>> + sample_size = 32;
>> + break;
>> default:
>> return -EINVAL;
>> }
>> @@ -170,8 +181,8 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> cif_conf.threshold = 0;
>> cif_conf.audio_channels = 2;
>> cif_conf.client_channels = 2;
>> - cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>> - cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
>> + cif_conf.audio_bits = audio_bits;
>> + cif_conf.client_bits = audio_bits;
>> cif_conf.expand = 0;
>> cif_conf.stereo_conv = 0;
>> cif_conf.replicate = 0;
>> @@ -264,10 +275,6 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask = 0, val = 0;
>> - dev_info(dai->dev, "%s: setting TDM: tx_mask: 0x%08x rx_mask:
>> 0x%08x "
>> - "slots: 0x%08x " "width: %d\n",
>> - __func__, tx_mask, rx_mask, slots, slot_width);
>> -
>
> This should be squashed in the previous patch
Thanks, looks like I missed a bad rebase and hit this patch instead of
the
previous.
>
>> /* Set up slots and tx/rx masks */
>> mask = TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_MASK |
>> TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_MASK |
>> @@ -277,6 +284,8 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> (rx_mask << TEGRA30_I2S_SLOT_CTRL_RX_SLOT_ENABLES_SHIFT) |
>> ((slots - 1) << TEGRA30_I2S_SLOT_CTRL_TOTAL_SLOTS_SHIFT);
>> + pm_runtime_get_sync(dai->dev);
>> +
>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_SLOT_CTRL, mask, val);
>> /* Set FSYNC width */
>> @@ -284,6 +293,7 @@ static int tegra30_i2s_set_tdm(struct snd_soc_dai
>> *dai,
>> TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_MASK,
>> (slot_width - 1) << TEGRA30_I2S_CH_CTRL_FSYNC_WIDTH_SHIFT);
>> + pm_runtime_put(dai->dev);
>
> same for PM stuff, it's not related to 24/32 bit samples
Yes, will do.
Thank you for reviewing.
>> return 0;
>> }
>> @@ -311,14 +321,18 @@ static const struct snd_soc_dai_driver
>> tegra30_i2s_dai_template = {
>> .channels_min = 2,
>> .channels_max = 2,
>> .rates = SNDRV_PCM_RATE_8000_96000,
>> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
>> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE |
>> + SNDRV_PCM_FMTBIT_S16_LE,
>> },
>> .capture = {
>> .stream_name = "Capture",
>> .channels_min = 2,
>> .channels_max = 2,
>> .rates = SNDRV_PCM_RATE_8000_96000,
>> - .formats = SNDRV_PCM_FMTBIT_S16_LE,
>> + .formats = SNDRV_PCM_FMTBIT_S32_LE |
>> + SNDRV_PCM_FMTBIT_S24_LE |
>> + SNDRV_PCM_FMTBIT_S16_LE,
>> },
>> .ops = &tegra30_i2s_dai_ops,
>> .symmetric_rates = 1,
>>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-18 7:44 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 10:08 ` Mark Brown
-1 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 10:08 UTC (permalink / raw)
To: Ben Dooks
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]
On Wed, Sep 18, 2019 at 08:44:50AM +0100, Ben Dooks wrote:
> On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
> > You need to add your own Signed-off-by when sending patches from other
> > people
> Yes, will do when this series has been reviewed and modifications done.
I didn't look at it due to the lack of signoffs.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-18 10:08 ` Mark Brown
0 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 10:08 UTC (permalink / raw)
To: Ben Dooks
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 322 bytes --]
On Wed, Sep 18, 2019 at 08:44:50AM +0100, Ben Dooks wrote:
> On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
> > You need to add your own Signed-off-by when sending patches from other
> > people
> Yes, will do when this series has been reviewed and modifications done.
I didn't look at it due to the lack of signoffs.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-18 10:08 ` [alsa-devel] " Mark Brown
@ 2019-09-18 11:50 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
On 2019-09-18 11:08, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 08:44:50AM +0100, Ben Dooks wrote:
>> On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
>
>> > You need to add your own Signed-off-by when sending patches from other
>> > people
>
>> Yes, will do when this series has been reviewed and modifications
>> done.
>
> I didn't look at it due to the lack of signoffs.
Thanks, although you could have just flagged this and reviewed the rest
anyway. I'll post a new version with signoff sorted at the end of the
week
as I cannot get in to the office to test any changes until Friday.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-18 11:50 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:50 UTC (permalink / raw)
To: Mark Brown
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
On 2019-09-18 11:08, Mark Brown wrote:
> On Wed, Sep 18, 2019 at 08:44:50AM +0100, Ben Dooks wrote:
>> On 2019-09-17 19:26, Pierre-Louis Bossart wrote:
>
>> > You need to add your own Signed-off-by when sending patches from other
>> > people
>
>> Yes, will do when this series has been reviewed and modifications
>> done.
>
> I didn't look at it due to the lack of signoffs.
Thanks, although you could have just flagged this and reviewed the rest
anyway. I'll post a new version with signoff sorted at the end of the
week
as I cannot get in to the office to test any changes until Friday.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
2019-09-18 11:50 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 12:05 ` Mark Brown
-1 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 12:05 UTC (permalink / raw)
To: Ben Dooks
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 869 bytes --]
On Wed, Sep 18, 2019 at 12:50:13PM +0100, Ben Dooks wrote:
> On 2019-09-18 11:08, Mark Brown wrote:
> > > Yes, will do when this series has been reviewed and modifications
> > > done.
> > I didn't look at it due to the lack of signoffs.
> Thanks, although you could have just flagged this and reviewed the rest
> anyway. I'll post a new version with signoff sorted at the end of the week
> as I cannot get in to the office to test any changes until Friday.
None of the patches I looked at had signoffs, Pierre had already told
you about that problem and there were a bunch of other review comments
anyway before I saw the series so it was fairly clear that a new version
is needed anyway. Once you've got some review you shouldn't rely on
getting extra review explicitly since it's not generally useful to repeat
the same review comments others have already made.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 2/8] ASoC: tegra: Allow 24bit and 32bit samples
@ 2019-09-18 12:05 ` Mark Brown
0 siblings, 0 replies; 68+ messages in thread
From: Mark Brown @ 2019-09-18 12:05 UTC (permalink / raw)
To: Ben Dooks
Cc: Jonathan Hunter, Pierre-Louis Bossart, alsa-devel, Liam Girdwood,
linux-kernel, Takashi Iwai, Thierry Reding, Edward Cragg,
linux-tegra, linux-tegra-owner
[-- Attachment #1.1: Type: text/plain, Size: 869 bytes --]
On Wed, Sep 18, 2019 at 12:50:13PM +0100, Ben Dooks wrote:
> On 2019-09-18 11:08, Mark Brown wrote:
> > > Yes, will do when this series has been reviewed and modifications
> > > done.
> > I didn't look at it due to the lack of signoffs.
> Thanks, although you could have just flagged this and reviewed the rest
> anyway. I'll post a new version with signoff sorted at the end of the week
> as I cannot get in to the office to test any changes until Friday.
None of the patches I looked at had signoffs, Pierre had already told
you about that problem and there were a bunch of other review comments
anyway before I saw the series so it was fairly clear that a new version
is needed anyway. Once you've got some review you shouldn't rely on
getting extra review explicitly since it's not generally useful to repeat
the same review comments others have already made.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
The CIF configuration and clock setting is currently hard coded for 2
channels. Since the hardware is capable of supporting 1-8 channels add
support for reading the channel count from the supplied parameters to
allow for better TDM support. It seems the original implementation of this
driver was fixed at 2 channels for simplicity, and not implementing TDM.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
[ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
---
sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
sound/soc/tegra/tegra30_i2s.h | 1 +
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index b5372839f672..40bcc05a9dbb 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
return -EINVAL;
}
+ i2s->is_tdm = false;
mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
TEGRA30_I2S_CTRL_LRCK_MASK;
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_DSP_A:
+ i2s->is_tdm = true;
val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
break;
case SND_SOC_DAIFMT_DSP_B:
+ i2s->is_tdm = true;
val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
break;
@@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
struct device *dev = dai->dev;
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask, val, reg;
- int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
+ int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
struct tegra30_ahub_cif_conf cif_conf;
- if (params_channels(params) != 2)
+ channels = params_channels(params);
+ if (channels > 8)
+ return -EINVAL;
+ if (channels != 2 && !i2s->is_tdm)
return -EINVAL;
mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;
@@ -157,9 +163,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
srate = params_rate(params);
-
/* Final "* 2" required by Tegra hardware */
- i2sclock = srate * params_channels(params) * sample_size * 2;
+ i2sclock = srate * channels * sample_size * 2;
bitcnt = (i2sclock / (2 * srate)) - 1;
if (bitcnt < 0 || bitcnt > TEGRA30_I2S_TIMING_CHANNEL_BIT_COUNT_MASK_US)
@@ -179,8 +184,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
regmap_write(i2s->regmap, TEGRA30_I2S_TIMING, val);
cif_conf.threshold = 0;
- cif_conf.audio_channels = 2;
- cif_conf.client_channels = 2;
+ cif_conf.audio_channels = channels;
+ cif_conf.client_channels = channels;
cif_conf.audio_bits = audio_bits;
cif_conf.client_bits = audio_bits;
cif_conf.expand = 0;
@@ -319,7 +324,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.playback = {
.stream_name = "Playback",
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_96000,
.formats = SNDRV_PCM_FMTBIT_S32_LE |
SNDRV_PCM_FMTBIT_S24_LE |
@@ -328,7 +333,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.capture = {
.stream_name = "Capture",
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_96000,
.formats = SNDRV_PCM_FMTBIT_S32_LE |
SNDRV_PCM_FMTBIT_S24_LE |
diff --git a/sound/soc/tegra/tegra30_i2s.h b/sound/soc/tegra/tegra30_i2s.h
index 0b1f3125a7c0..ae30e3c96337 100644
--- a/sound/soc/tegra/tegra30_i2s.h
+++ b/sound/soc/tegra/tegra30_i2s.h
@@ -224,6 +224,7 @@ struct tegra30_i2s {
const struct tegra30_i2s_soc_data *soc_data;
struct snd_soc_dai_driver dai;
int cif_id;
+ bool is_tdm;
struct clk *clk_i2s;
enum tegra30_ahub_txcif capture_i2s_cif;
enum tegra30_ahub_rxcif capture_fifo_cif;
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread* [alsa-devel] [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Edward Cragg
From: Edward Cragg <edward.cragg@codethink.co.uk>
The CIF configuration and clock setting is currently hard coded for 2
channels. Since the hardware is capable of supporting 1-8 channels add
support for reading the channel count from the supplied parameters to
allow for better TDM support. It seems the original implementation of this
driver was fixed at 2 channels for simplicity, and not implementing TDM.
Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
[ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
---
sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
sound/soc/tegra/tegra30_i2s.h | 1 +
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index b5372839f672..40bcc05a9dbb 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
return -EINVAL;
}
+ i2s->is_tdm = false;
mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
TEGRA30_I2S_CTRL_LRCK_MASK;
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_DSP_A:
+ i2s->is_tdm = true;
val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
break;
case SND_SOC_DAIFMT_DSP_B:
+ i2s->is_tdm = true;
val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
break;
@@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
struct device *dev = dai->dev;
struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
unsigned int mask, val, reg;
- int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
+ int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
struct tegra30_ahub_cif_conf cif_conf;
- if (params_channels(params) != 2)
+ channels = params_channels(params);
+ if (channels > 8)
+ return -EINVAL;
+ if (channels != 2 && !i2s->is_tdm)
return -EINVAL;
mask = TEGRA30_I2S_CTRL_BIT_SIZE_MASK;
@@ -157,9 +163,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
srate = params_rate(params);
-
/* Final "* 2" required by Tegra hardware */
- i2sclock = srate * params_channels(params) * sample_size * 2;
+ i2sclock = srate * channels * sample_size * 2;
bitcnt = (i2sclock / (2 * srate)) - 1;
if (bitcnt < 0 || bitcnt > TEGRA30_I2S_TIMING_CHANNEL_BIT_COUNT_MASK_US)
@@ -179,8 +184,8 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
regmap_write(i2s->regmap, TEGRA30_I2S_TIMING, val);
cif_conf.threshold = 0;
- cif_conf.audio_channels = 2;
- cif_conf.client_channels = 2;
+ cif_conf.audio_channels = channels;
+ cif_conf.client_channels = channels;
cif_conf.audio_bits = audio_bits;
cif_conf.client_bits = audio_bits;
cif_conf.expand = 0;
@@ -319,7 +324,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.playback = {
.stream_name = "Playback",
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_96000,
.formats = SNDRV_PCM_FMTBIT_S32_LE |
SNDRV_PCM_FMTBIT_S24_LE |
@@ -328,7 +333,7 @@ static const struct snd_soc_dai_driver tegra30_i2s_dai_template = {
.capture = {
.stream_name = "Capture",
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_96000,
.formats = SNDRV_PCM_FMTBIT_S32_LE |
SNDRV_PCM_FMTBIT_S24_LE |
diff --git a/sound/soc/tegra/tegra30_i2s.h b/sound/soc/tegra/tegra30_i2s.h
index 0b1f3125a7c0..ae30e3c96337 100644
--- a/sound/soc/tegra/tegra30_i2s.h
+++ b/sound/soc/tegra/tegra30_i2s.h
@@ -224,6 +224,7 @@ struct tegra30_i2s {
const struct tegra30_i2s_soc_data *soc_data;
struct snd_soc_dai_driver dai;
int cif_id;
+ bool is_tdm;
struct clk *clk_i2s;
enum tegra30_ahub_txcif capture_i2s_cif;
enum tegra30_ahub_rxcif capture_fifo_cif;
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 8:50 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:50 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Edward Cragg
On 17/09/2019 19:12, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> The CIF configuration and clock setting is currently hard coded for 2
> channels. Since the hardware is capable of supporting 1-8 channels add
> support for reading the channel count from the supplied parameters to
> allow for better TDM support. It seems the original implementation of this
> driver was fixed at 2 channels for simplicity, and not implementing TDM.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> [ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
> ---
> sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
> sound/soc/tegra/tegra30_i2s.h | 1 +
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index b5372839f672..40bcc05a9dbb 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
> return -EINVAL;
> }
>
> + i2s->is_tdm = false;
> mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
> TEGRA30_I2S_CTRL_LRCK_MASK;
> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> case SND_SOC_DAIFMT_DSP_A:
> + i2s->is_tdm = true;
> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
> val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
> break;
> case SND_SOC_DAIFMT_DSP_B:
> + i2s->is_tdm = true;
> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
> val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
> break;
> @@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> struct device *dev = dai->dev;
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask, val, reg;
> - int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
> struct tegra30_ahub_cif_conf cif_conf;
>
> - if (params_channels(params) != 2)
> + channels = params_channels(params);
> + if (channels > 8)
> + return -EINVAL;
> + if (channels != 2 && !i2s->is_tdm)
I don't think that this additional test is really necessary. I would
just drop this 'is_tdm' variable.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
@ 2019-09-18 8:50 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:50 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Edward Cragg
On 17/09/2019 19:12, Ben Dooks wrote:
> From: Edward Cragg <edward.cragg@codethink.co.uk>
>
> The CIF configuration and clock setting is currently hard coded for 2
> channels. Since the hardware is capable of supporting 1-8 channels add
> support for reading the channel count from the supplied parameters to
> allow for better TDM support. It seems the original implementation of this
> driver was fixed at 2 channels for simplicity, and not implementing TDM.
>
> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
> [ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
> ---
> sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
> sound/soc/tegra/tegra30_i2s.h | 1 +
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index b5372839f672..40bcc05a9dbb 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
> return -EINVAL;
> }
>
> + i2s->is_tdm = false;
> mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
> TEGRA30_I2S_CTRL_LRCK_MASK;
> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> case SND_SOC_DAIFMT_DSP_A:
> + i2s->is_tdm = true;
> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
> val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
> break;
> case SND_SOC_DAIFMT_DSP_B:
> + i2s->is_tdm = true;
> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
> val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
> break;
> @@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
> struct device *dev = dai->dev;
> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int mask, val, reg;
> - int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
> struct tegra30_ahub_cif_conf cif_conf;
>
> - if (params_channels(params) != 2)
> + channels = params_channels(params);
> + if (channels > 8)
> + return -EINVAL;
> + if (channels != 2 && !i2s->is_tdm)
I don't think that this additional test is really necessary. I would
just drop this 'is_tdm' variable.
Cheers
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
2019-09-18 8:50 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 10:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:12 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 2019-09-18 09:50, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> The CIF configuration and clock setting is currently hard coded for 2
>> channels. Since the hardware is capable of supporting 1-8 channels add
>> support for reading the channel count from the supplied parameters to
>> allow for better TDM support. It seems the original implementation of
>> this
>> driver was fixed at 2 channels for simplicity, and not implementing
>> TDM.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> [ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
>> sound/soc/tegra/tegra30_i2s.h | 1 +
>> 2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index b5372839f672..40bcc05a9dbb 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai
>> *dai,
>> return -EINVAL;
>> }
>>
>> + i2s->is_tdm = false;
>> mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
>> TEGRA30_I2S_CTRL_LRCK_MASK;
>> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>> case SND_SOC_DAIFMT_DSP_A:
>> + i2s->is_tdm = true;
>> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
>> val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
>> break;
>> case SND_SOC_DAIFMT_DSP_B:
>> + i2s->is_tdm = true;
>> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
>> val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
>> break;
>> @@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> struct device *dev = dai->dev;
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask, val, reg;
>> - int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
>> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
>> struct tegra30_ahub_cif_conf cif_conf;
>>
>> - if (params_channels(params) != 2)
>> + channels = params_channels(params);
>> + if (channels > 8)
>> + return -EINVAL;
>> + if (channels != 2 && !i2s->is_tdm)
>
> I don't think that this additional test is really necessary. I would
> just drop this 'is_tdm' variable.
I needed it elsewhere so would prefer to leave this here.
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 3/8] ASoC: tegra: i2s: Add support for more than 2 channels
@ 2019-09-18 10:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:12 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, Edward Cragg, linux-tegra,
linux-tegra-owner
On 2019-09-18 09:50, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> From: Edward Cragg <edward.cragg@codethink.co.uk>
>>
>> The CIF configuration and clock setting is currently hard coded for 2
>> channels. Since the hardware is capable of supporting 1-8 channels add
>> support for reading the channel count from the supplied parameters to
>> allow for better TDM support. It seems the original implementation of
>> this
>> driver was fixed at 2 channels for simplicity, and not implementing
>> TDM.
>>
>> Signed-off-by: Edward Cragg <edward.cragg@codethink.co.uk>
>> [ben.dooks@codethink.co.uk: added is_tdm and channel nr check]
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 21 +++++++++++++--------
>> sound/soc/tegra/tegra30_i2s.h | 1 +
>> 2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index b5372839f672..40bcc05a9dbb 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -86,14 +86,17 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai
>> *dai,
>> return -EINVAL;
>> }
>>
>> + i2s->is_tdm = false;
>> mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
>> TEGRA30_I2S_CTRL_LRCK_MASK;
>> switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>> case SND_SOC_DAIFMT_DSP_A:
>> + i2s->is_tdm = true;
>> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
>> val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
>> break;
>> case SND_SOC_DAIFMT_DSP_B:
>> + i2s->is_tdm = true;
>> val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
>> val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
>> break;
>> @@ -127,10 +130,13 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>> struct device *dev = dai->dev;
>> struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>> unsigned int mask, val, reg;
>> - int ret, sample_size, srate, i2sclock, bitcnt, audio_bits;
>> + int ret, sample_size, srate, i2sclock, bitcnt, audio_bits, channels;
>> struct tegra30_ahub_cif_conf cif_conf;
>>
>> - if (params_channels(params) != 2)
>> + channels = params_channels(params);
>> + if (channels > 8)
>> + return -EINVAL;
>> + if (channels != 2 && !i2s->is_tdm)
>
> I don't think that this additional test is really necessary. I would
> just drop this 'is_tdm' variable.
I needed it elsewhere so would prefer to leave this here.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 4/8] ASoC: tegra: disable rx_fifo after disable stream
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
We see odd FIFO overruns with this, we assume the best thing to do is
to disable the RX I2S frontend first, and then disable the FIFO that
is using it.
This also fixes an issue where using multi-word frames (TDM) have
partial samples stuck in the FIFO which then get read out when the
next capture is started.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 40bcc05a9dbb..4222839b63bd 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -236,9 +236,9 @@ static void tegra30_i2s_start_capture(struct tegra30_i2s *i2s)
static void tegra30_i2s_stop_capture(struct tegra30_i2s *i2s)
{
- tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL,
TEGRA30_I2S_CTRL_XFER_EN_RX, 0);
+ tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
}
static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread* [alsa-devel] [PATCH 4/8] ASoC: tegra: disable rx_fifo after disable stream
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
We see odd FIFO overruns with this, we assume the best thing to do is
to disable the RX I2S frontend first, and then disable the FIFO that
is using it.
This also fixes an issue where using multi-word frames (TDM) have
partial samples stuck in the FIFO which then get read out when the
next capture is started.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 40bcc05a9dbb..4222839b63bd 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -236,9 +236,9 @@ static void tegra30_i2s_start_capture(struct tegra30_i2s *i2s)
static void tegra30_i2s_stop_capture(struct tegra30_i2s *i2s)
{
- tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL,
TEGRA30_I2S_CTRL_XFER_EN_RX, 0);
+ tegra30_ahub_disable_rx_fifo(i2s->capture_fifo_cif);
}
static int tegra30_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
In TDM, use the negative edge to drive data and the positive edge to sample
data.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 4222839b63bd..d75ce12fe177 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
}
pm_runtime_get_sync(dai->dev);
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
+ i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
pm_runtime_put(dai->dev);
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [alsa-devel] [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
In TDM, use the negative edge to drive data and the positive edge to sample
data.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 4222839b63bd..d75ce12fe177 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
}
pm_runtime_get_sync(dai->dev);
+ regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
+ i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
pm_runtime_put(dai->dev);
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 19:33 ` kbuild test robot
-1 siblings, 0 replies; 68+ messages in thread
From: kbuild test robot @ 2019-09-17 19:33 UTC (permalink / raw)
Cc: Jonathan Hunter, linux-kernel, alsa-devel, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, kbuild-all,
linux-tegra, Ben Dooks
[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]
Hi Ben,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ben-Dooks/ASoC-tegra-Add-a-TDM-configuration-callback/20190918-022251
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/tegra/tegra30_i2s.c: In function 'tegra30_i2s_set_fmt':
>> sound/soc/tegra/tegra30_i2s.c:121:63: error: macro "regmap_update_bits" requires 4 arguments, but only 3 given
i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
^
>> sound/soc/tegra/tegra30_i2s.c:120:2: error: 'regmap_update_bits' undeclared (first use in this function); did you mean 'regmap_update_bits_base'?
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
^~~~~~~~~~~~~~~~~~
regmap_update_bits_base
sound/soc/tegra/tegra30_i2s.c:120:2: note: each undeclared identifier is reported only once for each function it appears in
vim +/regmap_update_bits +121 sound/soc/tegra/tegra30_i2s.c
64
65 static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
66 unsigned int fmt)
67 {
68 struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
69 unsigned int mask = 0, val = 0;
70
71 switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
72 case SND_SOC_DAIFMT_NB_NF:
73 break;
74 default:
75 return -EINVAL;
76 }
77
78 mask |= TEGRA30_I2S_CTRL_MASTER_ENABLE;
79 switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
80 case SND_SOC_DAIFMT_CBS_CFS:
81 val |= TEGRA30_I2S_CTRL_MASTER_ENABLE;
82 break;
83 case SND_SOC_DAIFMT_CBM_CFM:
84 break;
85 default:
86 return -EINVAL;
87 }
88
89 i2s->is_tdm = false;
90 mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
91 TEGRA30_I2S_CTRL_LRCK_MASK;
92 switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
93 case SND_SOC_DAIFMT_DSP_A:
94 i2s->is_tdm = true;
95 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
96 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
97 break;
98 case SND_SOC_DAIFMT_DSP_B:
99 i2s->is_tdm = true;
100 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
101 val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
102 break;
103 case SND_SOC_DAIFMT_I2S:
104 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
105 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
106 break;
107 case SND_SOC_DAIFMT_RIGHT_J:
108 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
109 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
110 break;
111 case SND_SOC_DAIFMT_LEFT_J:
112 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
113 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
114 break;
115 default:
116 return -EINVAL;
117 }
118
119 pm_runtime_get_sync(dai->dev);
> 120 regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
> 121 i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
122 regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
123 pm_runtime_put(dai->dev);
124
125 return 0;
126 }
127
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61579 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
@ 2019-09-17 19:33 ` kbuild test robot
0 siblings, 0 replies; 68+ messages in thread
From: kbuild test robot @ 2019-09-17 19:33 UTC (permalink / raw)
To: Ben Dooks
Cc: Jonathan Hunter, linux-kernel, alsa-devel, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, kbuild-all,
linux-tegra, Ben Dooks
[-- Attachment #1: Type: text/plain, Size: 3832 bytes --]
Hi Ben,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ben-Dooks/ASoC-tegra-Add-a-TDM-configuration-callback/20190918-022251
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/tegra/tegra30_i2s.c: In function 'tegra30_i2s_set_fmt':
>> sound/soc/tegra/tegra30_i2s.c:121:63: error: macro "regmap_update_bits" requires 4 arguments, but only 3 given
i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
^
>> sound/soc/tegra/tegra30_i2s.c:120:2: error: 'regmap_update_bits' undeclared (first use in this function); did you mean 'regmap_update_bits_base'?
regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
^~~~~~~~~~~~~~~~~~
regmap_update_bits_base
sound/soc/tegra/tegra30_i2s.c:120:2: note: each undeclared identifier is reported only once for each function it appears in
vim +/regmap_update_bits +121 sound/soc/tegra/tegra30_i2s.c
64
65 static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
66 unsigned int fmt)
67 {
68 struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai);
69 unsigned int mask = 0, val = 0;
70
71 switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
72 case SND_SOC_DAIFMT_NB_NF:
73 break;
74 default:
75 return -EINVAL;
76 }
77
78 mask |= TEGRA30_I2S_CTRL_MASTER_ENABLE;
79 switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
80 case SND_SOC_DAIFMT_CBS_CFS:
81 val |= TEGRA30_I2S_CTRL_MASTER_ENABLE;
82 break;
83 case SND_SOC_DAIFMT_CBM_CFM:
84 break;
85 default:
86 return -EINVAL;
87 }
88
89 i2s->is_tdm = false;
90 mask |= TEGRA30_I2S_CTRL_FRAME_FORMAT_MASK |
91 TEGRA30_I2S_CTRL_LRCK_MASK;
92 switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
93 case SND_SOC_DAIFMT_DSP_A:
94 i2s->is_tdm = true;
95 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
96 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
97 break;
98 case SND_SOC_DAIFMT_DSP_B:
99 i2s->is_tdm = true;
100 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_FSYNC;
101 val |= TEGRA30_I2S_CTRL_LRCK_R_LOW;
102 break;
103 case SND_SOC_DAIFMT_I2S:
104 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
105 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
106 break;
107 case SND_SOC_DAIFMT_RIGHT_J:
108 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
109 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
110 break;
111 case SND_SOC_DAIFMT_LEFT_J:
112 val |= TEGRA30_I2S_CTRL_FRAME_FORMAT_LRCK;
113 val |= TEGRA30_I2S_CTRL_LRCK_L_LOW;
114 break;
115 default:
116 return -EINVAL;
117 }
118
119 pm_runtime_get_sync(dai->dev);
> 120 regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
> 121 i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
122 regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
123 pm_runtime_put(dai->dev);
124
125 return 0;
126 }
127
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61579 bytes --]
[-- Attachment #3: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 8:54 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:54 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 17/09/2019 19:12, Ben Dooks wrote:
> In TDM, use the negative edge to drive data and the positive edge to sample
> data.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index 4222839b63bd..d75ce12fe177 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
> }
>
> pm_runtime_get_sync(dai->dev);
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
> pm_runtime_put(dai->dev);
I would rather set this in the case statement above where the format is
parsed and again drop this 'is_tdm' variable.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
@ 2019-09-18 8:54 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 8:54 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 17/09/2019 19:12, Ben Dooks wrote:
> In TDM, use the negative edge to drive data and the positive edge to sample
> data.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index 4222839b63bd..d75ce12fe177 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
> }
>
> pm_runtime_get_sync(dai->dev);
> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
> pm_runtime_put(dai->dev);
I would rather set this in the case statement above where the format is
parsed and again drop this 'is_tdm' variable.
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
2019-09-18 8:54 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 9:02 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:02 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 18/09/2019 09:54, Jon Hunter wrote:
>
> On 17/09/2019 19:12, Ben Dooks wrote:
>> In TDM, use the negative edge to drive data and the positive edge to sample
>> data.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
>> index 4222839b63bd..d75ce12fe177 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
>> }
>>
>> pm_runtime_get_sync(dai->dev);
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
>> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
>> pm_runtime_put(dai->dev);
>
> I would rather set this in the case statement above where the format is
> parsed and again drop this 'is_tdm' variable.
Actually, this should be implemented as shown in the following ...
https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l362
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
@ 2019-09-18 9:02 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:02 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 18/09/2019 09:54, Jon Hunter wrote:
>
> On 17/09/2019 19:12, Ben Dooks wrote:
>> In TDM, use the negative edge to drive data and the positive edge to sample
>> data.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
>> index 4222839b63bd..d75ce12fe177 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai,
>> }
>>
>> pm_runtime_get_sync(dai->dev);
>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
>> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
>> pm_runtime_put(dai->dev);
>
> I would rather set this in the case statement above where the format is
> parsed and again drop this 'is_tdm' variable.
Actually, this should be implemented as shown in the following ...
https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l362
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
2019-09-18 9:02 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 10:15 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:15 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, linux-tegra, linux-tegra-owner
On 2019-09-18 10:02, Jon Hunter wrote:
> On 18/09/2019 09:54, Jon Hunter wrote:
>>
>> On 17/09/2019 19:12, Ben Dooks wrote:
>>> In TDM, use the negative edge to drive data and the positive edge to
>>> sample
>>> data.
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>> ---
>>> sound/soc/tegra/tegra30_i2s.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>> b/sound/soc/tegra/tegra30_i2s.c
>>> index 4222839b63bd..d75ce12fe177 100644
>>> --- a/sound/soc/tegra/tegra30_i2s.c
>>> +++ b/sound/soc/tegra/tegra30_i2s.c
>>> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai
>>> *dai,
>>> }
>>>
>>> pm_runtime_get_sync(dai->dev);
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
>>> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
>>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
>>> pm_runtime_put(dai->dev);
>>
>> I would rather set this in the case statement above where the format
>> is
>> parsed and again drop this 'is_tdm' variable.
>
> Actually, this should be implemented as shown in the following ...
>
> https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l362
>
> Jon
Ok, will look at that.
Note, nv-tegra.nvidia.com seems to have a security problem .
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 5/8] ASoC: tegra: set edge mode for TDM correctly
@ 2019-09-18 10:15 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 10:15 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, linux-tegra, linux-tegra-owner
On 2019-09-18 10:02, Jon Hunter wrote:
> On 18/09/2019 09:54, Jon Hunter wrote:
>>
>> On 17/09/2019 19:12, Ben Dooks wrote:
>>> In TDM, use the negative edge to drive data and the positive edge to
>>> sample
>>> data.
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>> ---
>>> sound/soc/tegra/tegra30_i2s.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>> b/sound/soc/tegra/tegra30_i2s.c
>>> index 4222839b63bd..d75ce12fe177 100644
>>> --- a/sound/soc/tegra/tegra30_i2s.c
>>> +++ b/sound/soc/tegra/tegra30_i2s.c
>>> @@ -117,6 +117,8 @@ static int tegra30_i2s_set_fmt(struct snd_soc_dai
>>> *dai,
>>> }
>>>
>>> pm_runtime_get_sync(dai->dev);
>>> + regmap_update_bits(i2s->regmap, TEGRA30_I2S_CH_CTRL_EGDE_CTRL_MASK,
>>> + i2s->is_tdm ? TEGRA30_I2S_CH_CTRL_EGDE_CTRL_NEG_EDGE : 0);
>>> regmap_update_bits(i2s->regmap, TEGRA30_I2S_CTRL, mask, val);
>>> pm_runtime_put(dai->dev);
>>
>> I would rather set this in the case statement above where the format
>> is
>> parsed and again drop this 'is_tdm' variable.
>
> Actually, this should be implemented as shown in the following ...
>
> https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l362
>
> Jon
Ok, will look at that.
Note, nv-tegra.nvidia.com seems to have a security problem .
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
Set the offset to 0 for TDM mode, as per the current setup.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index d75ce12fe177..3efef87ed8d8 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
- val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
- (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
+ if (i2s->is_tdm)
+ val = 0;
+ else
+ val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
+ (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
return 0;
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [alsa-devel] [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
Set the offset to 0 for TDM mode, as per the current setup.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_i2s.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index d75ce12fe177..3efef87ed8d8 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
- val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
- (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
+ if (i2s->is_tdm)
+ val = 0;
+ else
+ val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
+ (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
return 0;
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 9:02 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:02 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 17/09/2019 19:12, Ben Dooks wrote:
> Set the offset to 0 for TDM mode, as per the current setup.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index d75ce12fe177..3efef87ed8d8 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>
> i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
>
> - val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
> - (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
> + if (i2s->is_tdm)
> + val = 0;
> + else
> + val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
> + (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
> regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
>
> return 0;
Please move this code into tegra30_i2s_set_fmt() as it only needs to be
set once.
BTW, if you refer to the following I2S driver for Tegra210, you will see
how I think that we should handle this ...
https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l310
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
@ 2019-09-18 9:02 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:02 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 17/09/2019 19:12, Ben Dooks wrote:
> Set the offset to 0 for TDM mode, as per the current setup.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_i2s.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index d75ce12fe177..3efef87ed8d8 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
>
> i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
>
> - val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
> - (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
> + if (i2s->is_tdm)
> + val = 0;
> + else
> + val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
> + (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
> regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
>
> return 0;
Please move this code into tegra30_i2s_set_fmt() as it only needs to be
set once.
BTW, if you refer to the following I2S driver for Tegra210, you will see
how I think that we should handle this ...
https://nv-tegra.nvidia.com/gitweb/?p=linux-nvidia.git;a=blob;f=sound/soc/tegra-alt/tegra210_i2s_alt.c;h=ee482f27ed2e4710e5e7446918887f8f8ef31285;hb=a960d522a5486aee27605f890034869c4f49d94a#l310
Cheers
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
2019-09-18 9:02 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 11:30 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:30 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, linux-tegra
On 2019-09-18 10:02, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> Set the offset to 0 for TDM mode, as per the current setup.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index d75ce12fe177..3efef87ed8d8 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>>
>> i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
>>
>> - val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
>> - (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
>> + if (i2s->is_tdm)
>> + val = 0;
>> + else
>> + val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
>> + (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
>> regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
>>
>> return 0;
>
> Please move this code into tegra30_i2s_set_fmt() as it only needs to be
> set once.
>
> BTW, if you refer to the following I2S driver for Tegra210, you will
> see
> how I think that we should handle this ...
Ok, thanks.
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 6/8] ASoC: tegra: set i2s_offset to 0 for tdm
@ 2019-09-18 11:30 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:30 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Liam Girdwood, Takashi Iwai,
Mark Brown, Thierry Reding, linux-tegra
On 2019-09-18 10:02, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> Set the offset to 0 for TDM mode, as per the current setup.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_i2s.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>> b/sound/soc/tegra/tegra30_i2s.c
>> index d75ce12fe177..3efef87ed8d8 100644
>> --- a/sound/soc/tegra/tegra30_i2s.c
>> +++ b/sound/soc/tegra/tegra30_i2s.c
>> @@ -206,8 +206,11 @@ static int tegra30_i2s_hw_params(struct
>> snd_pcm_substream *substream,
>>
>> i2s->soc_data->set_audio_cif(i2s->regmap, reg, &cif_conf);
>>
>> - val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
>> - (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
>> + if (i2s->is_tdm)
>> + val = 0;
>> + else
>> + val = (1 << TEGRA30_I2S_OFFSET_RX_DATA_OFFSET_SHIFT) |
>> + (1 << TEGRA30_I2S_OFFSET_TX_DATA_OFFSET_SHIFT);
>> regmap_write(i2s->regmap, TEGRA30_I2S_OFFSET, val);
>>
>> return 0;
>
> Please move this code into tegra30_i2s_set_fmt() as it only needs to be
> set once.
>
> BTW, if you refer to the following I2S driver for Tegra210, you will
> see
> how I think that we should handle this ...
Ok, thanks.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
If the hw_params uses a different bit or channel count, then we
need to change both the I2S unit's CIF configuration as well as
the APBIF one.
To allow changing the APBIF, add a call to reconfigure the RX or
TX FIFO without changing the DMA or allocation, and get the I2S
driver to call it once the hw params have been calculate.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
sound/soc/tegra/tegra30_ahub.h | 5 ++
sound/soc/tegra/tegra30_i2s.c | 2 +
3 files changed, 69 insertions(+), 52 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 952381260dc3..58e05ceb86da 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
return 0;
}
+int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+ struct tegra30_ahub_cif_conf *cif_conf)
+{
+ int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+ u32 reg, val;
+
+ pm_runtime_get_sync(ahub->dev);
+
+ reg = TEGRA30_AHUB_CHANNEL_CTRL +
+ (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+ val = tegra30_apbif_read(reg);
+ val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ tegra30_apbif_write(reg, val);
+
+ cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+ reg = TEGRA30_AHUB_CIF_RX_CTRL +
+ (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+ ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+ pm_runtime_put(ahub->dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
char *dmachan, int dmachan_len,
dma_addr_t *fiforeg)
{
int channel;
- u32 reg, val;
struct tegra30_ahub_cif_conf cif_conf;
channel = find_first_zero_bit(ahub->rx_usage,
@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
(channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
- pm_runtime_get_sync(ahub->dev);
+ memset(&cif_conf, 0, sizeof(cif_conf));
- reg = TEGRA30_AHUB_CHANNEL_CTRL +
- (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
- val = tegra30_apbif_read(reg);
- val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
- tegra30_apbif_write(reg, val);
-
- cif_conf.threshold = 0;
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.expand = 0;
- cif_conf.stereo_conv = 0;
- cif_conf.replicate = 0;
- cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
- cif_conf.truncate = 0;
- cif_conf.mono_conv = 0;
-
- reg = TEGRA30_AHUB_CIF_RX_CTRL +
- (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
- ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
- pm_runtime_put(ahub->dev);
- return 0;
+ return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);
}
EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo);
@@ -186,12 +191,40 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif)
}
EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
+int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+ struct tegra30_ahub_cif_conf *cif_conf)
+{
+ int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0;
+ u32 reg, val;
+
+ pm_runtime_get_sync(ahub->dev);
+
+ reg = TEGRA30_AHUB_CHANNEL_CTRL +
+ (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+ val = tegra30_apbif_read(reg);
+ val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ tegra30_apbif_write(reg, val);
+
+ cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+
+ reg = TEGRA30_AHUB_CIF_TX_CTRL +
+ (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
+ ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+ pm_runtime_put(ahub->dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_tx_fifo);
+
int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
char *dmachan, int dmachan_len,
dma_addr_t *fiforeg)
{
int channel;
- u32 reg, val;
struct tegra30_ahub_cif_conf cif_conf;
channel = find_first_zero_bit(ahub->tx_usage,
@@ -206,37 +239,14 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO +
(channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE);
- pm_runtime_get_sync(ahub->dev);
-
- reg = TEGRA30_AHUB_CHANNEL_CTRL +
- (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
- val = tegra30_apbif_read(reg);
- val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
- tegra30_apbif_write(reg, val);
-
- cif_conf.threshold = 0;
+ memset(&cif_conf, 0, sizeof(cif_conf));
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.expand = 0;
- cif_conf.stereo_conv = 0;
- cif_conf.replicate = 0;
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
- cif_conf.truncate = 0;
- cif_conf.mono_conv = 0;
-
- reg = TEGRA30_AHUB_CIF_TX_CTRL +
- (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
- ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
- pm_runtime_put(ahub->dev);
-
- return 0;
+ return tegra30_ahub_setup_tx_fifo(*txcif, &cif_conf);
}
EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo);
diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h
index 6889c5f23d02..26120aee64b3 100644
--- a/sound/soc/tegra/tegra30_ahub.h
+++ b/sound/soc/tegra/tegra30_ahub.h
@@ -490,6 +490,11 @@ void tegra30_ahub_set_cif(struct regmap *regmap, unsigned int reg,
void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
struct tegra30_ahub_cif_conf *conf);
+extern int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+ struct tegra30_ahub_cif_conf *cif_conf);
+extern int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif,
+ struct tegra30_ahub_cif_conf *cif_conf);
+
struct tegra30_ahub_soc_data {
u32 mod_list_mask;
void (*set_audio_cif)(struct regmap *regmap,
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 3efef87ed8d8..805e0f4da52a 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -198,9 +198,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+ tegra30_ahub_setup_tx_fifo(i2s->playback_fifo_cif, &cif_conf);
reg = TEGRA30_I2S_CIF_RX_CTRL;
} else {
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+ tegra30_ahub_setup_rx_fifo(i2s->capture_fifo_cif, &cif_conf);
reg = TEGRA30_I2S_CIF_TX_CTRL;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread* [alsa-devel] [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
If the hw_params uses a different bit or channel count, then we
need to change both the I2S unit's CIF configuration as well as
the APBIF one.
To allow changing the APBIF, add a call to reconfigure the RX or
TX FIFO without changing the DMA or allocation, and get the I2S
driver to call it once the hw params have been calculate.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
sound/soc/tegra/tegra30_ahub.h | 5 ++
sound/soc/tegra/tegra30_i2s.c | 2 +
3 files changed, 69 insertions(+), 52 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 952381260dc3..58e05ceb86da 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
return 0;
}
+int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
+ struct tegra30_ahub_cif_conf *cif_conf)
+{
+ int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
+ u32 reg, val;
+
+ pm_runtime_get_sync(ahub->dev);
+
+ reg = TEGRA30_AHUB_CHANNEL_CTRL +
+ (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+ val = tegra30_apbif_read(reg);
+ val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ tegra30_apbif_write(reg, val);
+
+ cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+
+ reg = TEGRA30_AHUB_CIF_RX_CTRL +
+ (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
+ ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+ pm_runtime_put(ahub->dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_rx_fifo);
+
int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
char *dmachan, int dmachan_len,
dma_addr_t *fiforeg)
{
int channel;
- u32 reg, val;
struct tegra30_ahub_cif_conf cif_conf;
channel = find_first_zero_bit(ahub->rx_usage,
@@ -104,37 +132,14 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif,
*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO +
(channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE);
- pm_runtime_get_sync(ahub->dev);
+ memset(&cif_conf, 0, sizeof(cif_conf));
- reg = TEGRA30_AHUB_CHANNEL_CTRL +
- (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
- val = tegra30_apbif_read(reg);
- val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
- tegra30_apbif_write(reg, val);
-
- cif_conf.threshold = 0;
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.expand = 0;
- cif_conf.stereo_conv = 0;
- cif_conf.replicate = 0;
- cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
- cif_conf.truncate = 0;
- cif_conf.mono_conv = 0;
-
- reg = TEGRA30_AHUB_CIF_RX_CTRL +
- (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE);
- ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
-
- pm_runtime_put(ahub->dev);
- return 0;
+ return tegra30_ahub_setup_rx_fifo(*rxcif, &cif_conf);
}
EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo);
@@ -186,12 +191,40 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif)
}
EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
+int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+ struct tegra30_ahub_cif_conf *cif_conf)
+{
+ int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0;
+ u32 reg, val;
+
+ pm_runtime_get_sync(ahub->dev);
+
+ reg = TEGRA30_AHUB_CHANNEL_CTRL +
+ (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
+ val = tegra30_apbif_read(reg);
+ val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ tegra30_apbif_write(reg, val);
+
+ cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+
+ reg = TEGRA30_AHUB_CIF_TX_CTRL +
+ (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
+ ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, cif_conf);
+
+ pm_runtime_put(ahub->dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra30_ahub_setup_tx_fifo);
+
int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
char *dmachan, int dmachan_len,
dma_addr_t *fiforeg)
{
int channel;
- u32 reg, val;
struct tegra30_ahub_cif_conf cif_conf;
channel = find_first_zero_bit(ahub->tx_usage,
@@ -206,37 +239,14 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif,
*fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO +
(channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE);
- pm_runtime_get_sync(ahub->dev);
-
- reg = TEGRA30_AHUB_CHANNEL_CTRL +
- (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
- val = tegra30_apbif_read(reg);
- val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
- tegra30_apbif_write(reg, val);
-
- cif_conf.threshold = 0;
+ memset(&cif_conf, 0, sizeof(cif_conf));
cif_conf.audio_channels = 2;
cif_conf.client_channels = 2;
cif_conf.audio_bits = TEGRA30_AUDIOCIF_BITS_16;
cif_conf.client_bits = TEGRA30_AUDIOCIF_BITS_16;
- cif_conf.expand = 0;
- cif_conf.stereo_conv = 0;
- cif_conf.replicate = 0;
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
- cif_conf.truncate = 0;
- cif_conf.mono_conv = 0;
-
- reg = TEGRA30_AHUB_CIF_TX_CTRL +
- (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE);
- ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
- pm_runtime_put(ahub->dev);
-
- return 0;
+ return tegra30_ahub_setup_tx_fifo(*txcif, &cif_conf);
}
EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo);
diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h
index 6889c5f23d02..26120aee64b3 100644
--- a/sound/soc/tegra/tegra30_ahub.h
+++ b/sound/soc/tegra/tegra30_ahub.h
@@ -490,6 +490,11 @@ void tegra30_ahub_set_cif(struct regmap *regmap, unsigned int reg,
void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
struct tegra30_ahub_cif_conf *conf);
+extern int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
+ struct tegra30_ahub_cif_conf *cif_conf);
+extern int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif,
+ struct tegra30_ahub_cif_conf *cif_conf);
+
struct tegra30_ahub_soc_data {
u32 mod_list_mask;
void (*set_audio_cif)(struct regmap *regmap,
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index 3efef87ed8d8..805e0f4da52a 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -198,9 +198,11 @@ static int tegra30_i2s_hw_params(struct snd_pcm_substream *substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
+ tegra30_ahub_setup_tx_fifo(i2s->playback_fifo_cif, &cif_conf);
reg = TEGRA30_I2S_CIF_RX_CTRL;
} else {
cif_conf.direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
+ tegra30_ahub_setup_rx_fifo(i2s->capture_fifo_cif, &cif_conf);
reg = TEGRA30_I2S_CIF_TX_CTRL;
}
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 9:14 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:14 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Sameer Pujar
On 17/09/2019 19:12, Ben Dooks wrote:
> If the hw_params uses a different bit or channel count, then we
> need to change both the I2S unit's CIF configuration as well as
> the APBIF one.
>
> To allow changing the APBIF, add a call to reconfigure the RX or
> TX FIFO without changing the DMA or allocation, and get the I2S
> driver to call it once the hw params have been calculate.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
> sound/soc/tegra/tegra30_ahub.h | 5 ++
> sound/soc/tegra/tegra30_i2s.c | 2 +
> 3 files changed, 69 insertions(+), 52 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
> index 952381260dc3..58e05ceb86da 100644
> --- a/sound/soc/tegra/tegra30_ahub.c
> +++ b/sound/soc/tegra/tegra30_ahub.c
> @@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
> return 0;
> }
>
> +int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
> + struct tegra30_ahub_cif_conf *cif_conf)
> +{
> + int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
> + u32 reg, val;
> +
> + pm_runtime_get_sync(ahub->dev);
> +
> + reg = TEGRA30_AHUB_CHANNEL_CTRL +
> + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
> + val = tegra30_apbif_read(reg);
> + val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + tegra30_apbif_write(reg, val);
Aren't you just programming the same value to the APBIF here that was
previously programmed by the allocate function? I don't see the point in
moving this? What am I missing here?
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
@ 2019-09-18 9:14 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:14 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Sameer Pujar
On 17/09/2019 19:12, Ben Dooks wrote:
> If the hw_params uses a different bit or channel count, then we
> need to change both the I2S unit's CIF configuration as well as
> the APBIF one.
>
> To allow changing the APBIF, add a call to reconfigure the RX or
> TX FIFO without changing the DMA or allocation, and get the I2S
> driver to call it once the hw params have been calculate.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_ahub.c | 114 ++++++++++++++++++---------------
> sound/soc/tegra/tegra30_ahub.h | 5 ++
> sound/soc/tegra/tegra30_i2s.c | 2 +
> 3 files changed, 69 insertions(+), 52 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
> index 952381260dc3..58e05ceb86da 100644
> --- a/sound/soc/tegra/tegra30_ahub.c
> +++ b/sound/soc/tegra/tegra30_ahub.c
> @@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct device *dev)
> return 0;
> }
>
> +int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
> + struct tegra30_ahub_cif_conf *cif_conf)
> +{
> + int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
> + u32 reg, val;
> +
> + pm_runtime_get_sync(ahub->dev);
> +
> + reg = TEGRA30_AHUB_CHANNEL_CTRL +
> + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
> + val = tegra30_apbif_read(reg);
> + val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + tegra30_apbif_write(reg, val);
Aren't you just programming the same value to the APBIF here that was
previously programmed by the allocate function? I don't see the point in
moving this? What am I missing here?
Cheers
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
2019-09-18 9:14 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 11:41 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:41 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Sameer Pujar, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, linux-tegra
On 2019-09-18 10:14, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> If the hw_params uses a different bit or channel count, then we
>> need to change both the I2S unit's CIF configuration as well as
>> the APBIF one.
>>
>> To allow changing the APBIF, add a call to reconfigure the RX or
>> TX FIFO without changing the DMA or allocation, and get the I2S
>> driver to call it once the hw params have been calculate.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_ahub.c | 114
>> ++++++++++++++++++---------------
>> sound/soc/tegra/tegra30_ahub.h | 5 ++
>> sound/soc/tegra/tegra30_i2s.c | 2 +
>> 3 files changed, 69 insertions(+), 52 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_ahub.c
>> b/sound/soc/tegra/tegra30_ahub.c
>> index 952381260dc3..58e05ceb86da 100644
>> --- a/sound/soc/tegra/tegra30_ahub.c
>> +++ b/sound/soc/tegra/tegra30_ahub.c
>> @@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct
>> device *dev)
>> return 0;
>> }
>>
>> +int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
>> + struct tegra30_ahub_cif_conf *cif_conf)
>> +{
>> + int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
>> + u32 reg, val;
>> +
>> + pm_runtime_get_sync(ahub->dev);
>> +
>> + reg = TEGRA30_AHUB_CHANNEL_CTRL +
>> + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
>> + val = tegra30_apbif_read(reg);
>> + val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
>> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + tegra30_apbif_write(reg, val);
>
> Aren't you just programming the same value to the APBIF here that was
> previously programmed by the allocate function? I don't see the point
> in
> moving this? What am I missing here?
IIRC, this was due to trying different types of playback with a 4ch
32bit
sound output. The ahub and the i2s unit don't have to agreet.
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [alsa-devel] [PATCH 7/8] ASoC: tegra: config fifos on hw_param changes
@ 2019-09-18 11:41 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-18 11:41 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kernel, alsa-devel, Sameer Pujar, Liam Girdwood,
Takashi Iwai, Mark Brown, Thierry Reding, linux-tegra
On 2019-09-18 10:14, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> If the hw_params uses a different bit or channel count, then we
>> need to change both the I2S unit's CIF configuration as well as
>> the APBIF one.
>>
>> To allow changing the APBIF, add a call to reconfigure the RX or
>> TX FIFO without changing the DMA or allocation, and get the I2S
>> driver to call it once the hw params have been calculate.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_ahub.c | 114
>> ++++++++++++++++++---------------
>> sound/soc/tegra/tegra30_ahub.h | 5 ++
>> sound/soc/tegra/tegra30_i2s.c | 2 +
>> 3 files changed, 69 insertions(+), 52 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_ahub.c
>> b/sound/soc/tegra/tegra30_ahub.c
>> index 952381260dc3..58e05ceb86da 100644
>> --- a/sound/soc/tegra/tegra30_ahub.c
>> +++ b/sound/soc/tegra/tegra30_ahub.c
>> @@ -84,12 +84,40 @@ static int tegra30_ahub_runtime_resume(struct
>> device *dev)
>> return 0;
>> }
>>
>> +int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
>> + struct tegra30_ahub_cif_conf *cif_conf)
>> +{
>> + int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0;
>> + u32 reg, val;
>> +
>> + pm_runtime_get_sync(ahub->dev);
>> +
>> + reg = TEGRA30_AHUB_CHANNEL_CTRL +
>> + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
>> + val = tegra30_apbif_read(reg);
>> + val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
>> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + tegra30_apbif_write(reg, val);
>
> Aren't you just programming the same value to the APBIF here that was
> previously programmed by the allocate function? I don't see the point
> in
> moving this? What am I missing here?
IIRC, this was due to trying different types of playback with a 4ch
32bit
sound output. The ahub and the i2s unit don't have to agreet.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-17 18:12 ` Ben Dooks
-1 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
If the CIF is not configured as 16 or 8 bit, then the
packing for 8/16 bits should not be enabled as the
hardware only supports 8 or 16 bit packing.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 58e05ceb86da..c2f2e29dd32e 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
(channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
val = tegra30_apbif_read(reg);
val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+ cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
+
tegra30_apbif_write(reg, val);
cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
@@ -203,10 +210,16 @@ int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
(channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
val = tegra30_apbif_read(reg);
val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT);
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+ cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_8_4;
tegra30_apbif_write(reg, val);
cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
--
2.23.0
^ permalink raw reply related [flat|nested] 68+ messages in thread
* [alsa-devel] [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
@ 2019-09-17 18:12 ` Ben Dooks
0 siblings, 0 replies; 68+ messages in thread
From: Ben Dooks @ 2019-09-17 18:12 UTC (permalink / raw)
To: linux-tegra, alsa-devel, Jaroslav Kysela, Takashi Iwai,
Liam Girdwood, Mark Brown, Thierry Reding, Jonathan Hunter
Cc: linux-kernel, Ben Dooks
If the CIF is not configured as 16 or 8 bit, then the
packing for 8/16 bits should not be enabled as the
hardware only supports 8 or 16 bit packing.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 58e05ceb86da..c2f2e29dd32e 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
(channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
val = tegra30_apbif_read(reg);
val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+ cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
+
tegra30_apbif_write(reg, val);
cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_RX;
@@ -203,10 +210,16 @@ int tegra30_ahub_setup_tx_fifo(enum tegra30_ahub_txcif txcif,
(channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
val = tegra30_apbif_read(reg);
val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_MASK |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK);
- val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT) |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN |
- TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_MASK |
+ TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN);
+ val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_TX_THRESHOLD_SHIFT);
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
+ cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_EN;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_16;
+ if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
+ val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_PACK_8_4;
tegra30_apbif_write(reg, val);
cif_conf->direction = TEGRA30_AUDIOCIF_DIRECTION_TX;
--
2.23.0
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
2019-09-17 18:12 ` [alsa-devel] " Ben Dooks
@ 2019-09-18 9:16 ` Jon Hunter
-1 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:16 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Sameer Pujar
On 17/09/2019 19:12, Ben Dooks wrote:
> If the CIF is not configured as 16 or 8 bit, then the
> packing for 8/16 bits should not be enabled as the
> hardware only supports 8 or 16 bit packing.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
> index 58e05ceb86da..c2f2e29dd32e 100644
> --- a/sound/soc/tegra/tegra30_ahub.c
> +++ b/sound/soc/tegra/tegra30_ahub.c
> @@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
> (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
> val = tegra30_apbif_read(reg);
> val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
> - val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
> + cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
> +
Ah maybe this is what I am missing from the previous patch. So the last
patch was a preparatory patch for this one.
Sameer, how is this handled in the case of Tegra210?
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
@ 2019-09-18 9:16 ` Jon Hunter
0 siblings, 0 replies; 68+ messages in thread
From: Jon Hunter @ 2019-09-18 9:16 UTC (permalink / raw)
To: Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel, Sameer Pujar
On 17/09/2019 19:12, Ben Dooks wrote:
> If the CIF is not configured as 16 or 8 bit, then the
> packing for 8/16 bits should not be enabled as the
> hardware only supports 8 or 16 bit packing.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
> index 58e05ceb86da..c2f2e29dd32e 100644
> --- a/sound/soc/tegra/tegra30_ahub.c
> +++ b/sound/soc/tegra/tegra30_ahub.c
> @@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
> (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
> val = tegra30_apbif_read(reg);
> val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
> - val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
> + cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
> +
Ah maybe this is what I am missing from the previous patch. So the last
patch was a preparatory patch for this one.
Sameer, how is this handled in the case of Tegra210?
Cheers
Jon
--
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
2019-09-18 9:16 ` [alsa-devel] " Jon Hunter
@ 2019-09-18 10:02 ` Sameer Pujar
-1 siblings, 0 replies; 68+ messages in thread
From: Sameer Pujar @ 2019-09-18 10:02 UTC (permalink / raw)
To: Jon Hunter, Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 9/18/2019 2:46 PM, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> If the CIF is not configured as 16 or 8 bit, then the
>> packing for 8/16 bits should not be enabled as the
>> hardware only supports 8 or 16 bit packing.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
>> 1 file changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
>> index 58e05ceb86da..c2f2e29dd32e 100644
>> --- a/sound/soc/tegra/tegra30_ahub.c
>> +++ b/sound/soc/tegra/tegra30_ahub.c
>> @@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
>> (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
>> val = tegra30_apbif_read(reg);
>> val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
>> - val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
>> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
>> + cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
>> +
> Ah maybe this is what I am missing from the previous patch. So the last
> patch was a preparatory patch for this one.
>
> Sameer, how is this handled in the case of Tegra210?
For Tegra210 we have a separate driver for ADMAIF. Packing and CIF
configuration
is programmed in its hw_param() callback.
> Cheers
> Jon
>
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [alsa-devel] [PATCH 8/8] ASoC: tegra: take packing settings from the audio cif_config
@ 2019-09-18 10:02 ` Sameer Pujar
0 siblings, 0 replies; 68+ messages in thread
From: Sameer Pujar @ 2019-09-18 10:02 UTC (permalink / raw)
To: Jon Hunter, Ben Dooks, linux-tegra, alsa-devel, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown, Thierry Reding
Cc: linux-kernel
On 9/18/2019 2:46 PM, Jon Hunter wrote:
> On 17/09/2019 19:12, Ben Dooks wrote:
>> If the CIF is not configured as 16 or 8 bit, then the
>> packing for 8/16 bits should not be enabled as the
>> hardware only supports 8 or 16 bit packing.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> sound/soc/tegra/tegra30_ahub.c | 29 +++++++++++++++++++++--------
>> 1 file changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
>> index 58e05ceb86da..c2f2e29dd32e 100644
>> --- a/sound/soc/tegra/tegra30_ahub.c
>> +++ b/sound/soc/tegra/tegra30_ahub.c
>> @@ -96,10 +96,17 @@ int tegra30_ahub_setup_rx_fifo(enum tegra30_ahub_rxcif rxcif,
>> (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE);
>> val = tegra30_apbif_read(reg);
>> val &= ~(TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_MASK |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK);
>> - val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT) |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN |
>> - TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_MASK |
>> + TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN);
>> + val |= (7 << TEGRA30_AHUB_CHANNEL_CTRL_RX_THRESHOLD_SHIFT);
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16 ||
>> + cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_EN;
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_16)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_16;
>> + if (cif_conf->audio_bits == TEGRA30_AUDIOCIF_BITS_8)
>> + val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_PACK_8_4;
>> +
> Ah maybe this is what I am missing from the previous patch. So the last
> patch was a preparatory patch for this one.
>
> Sameer, how is this handled in the case of Tegra210?
For Tegra210 we have a separate driver for ADMAIF. Packing and CIF
configuration
is programmed in its hw_param() callback.
> Cheers
> Jon
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 68+ messages in thread