(unknown),

(unknown),

[Sangbeom Kim]

[PATCH 0/2] Add WM8994 PCM Machine driver for Exynos4

Hi,

This is patchset for WM8994 pcm machine driver that supports
PCM audio on SMDKV310, SMDKC210 and test is done.

Based on these patches WM8994 pcm machine driver supports followings:
 o Playback supports 8kHz sampling rates.
 o Capture supports 8kHz sampling rates.

This patchset contains followings 

 o To Kukjin Kim and Ben Dooks,
[PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994

 o To Jassi Brar, Mark Brown and Liam Girdwood,
[PATCH 2/2] ASoC: SAMSUNG: Add WM8580 PCM Machine driver 

Best Regards,
SB Kim (Sangbeom Kim)

[PATCH 2/2] ASoC: SAMSUNG: Add WM8994 PCM Machine driver

[Sangbeom Kim]

This patch add WM8994 PCM machine driver to support PCM audio
on SMDKV310, SMDKC210 boards.
Playback and Capture supports 8kHz sampling rates.
and It is tested on SMDKV310, SMDKC210.

Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
---
 sound/soc/samsung/Kconfig          |    8 ++
 sound/soc/samsung/Makefile         |    2 +
 sound/soc/samsung/smdk_wm8994pcm.c |  175 ++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/samsung/smdk_wm8994pcm.c

diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index b8c7a2e..db2bc49 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -170,3 +170,11 @@ config SND_SOC_SMDK_WM8580_PCM
 	select SND_SAMSUNG_PCM
 	help
 	  Say Y if you want to add support for SoC audio on the SMDK.
+
+config SND_SOC_SMDK_WM8994_PCM
+	tristate "SoC PCM Audio support for WM8994 on SMDK"
+	depends on SND_SOC_SAMSUNG && (MACH_SMDKC210 || MACH_SMDKV310)
+	select SND_SOC_WM8994
+	select SND_SAMSUNG_PCM
+	help
+	  Say Y if you want to add support for SoC audio on the SMDK.
diff --git a/sound/soc/samsung/Makefile b/sound/soc/samsung/Makefile
index 6c598fc..4a3a428 100644
--- a/sound/soc/samsung/Makefile
+++ b/sound/soc/samsung/Makefile
@@ -35,6 +35,7 @@ snd-soc-s3c64xx-smartq-wm8987-objs := smartq_wm8987.o
 snd-soc-goni-wm8994-objs := goni_wm8994.o
 snd-soc-smdk-spdif-objs := smdk_spdif.o
 snd-soc-smdk-wm8580pcm-objs := smdk_wm8580pcm.o
+snd-soc-smdk-wm8994pcm-objs := smdk_wm8994pcm.o
 
 obj-$(CONFIG_SND_SOC_SAMSUNG_JIVE_WM8750) += snd-soc-jive-wm8750.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_NEO1973_WM8753) += snd-soc-neo1973-wm8753.o
@@ -53,3 +54,4 @@ obj-$(CONFIG_SND_SOC_SMARTQ) += snd-soc-s3c64xx-smartq-wm8987.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_SPDIF) += snd-soc-smdk-spdif.o
 obj-$(CONFIG_SND_SOC_GONI_AQUILA_WM8994) += snd-soc-goni-wm8994.o
 obj-$(CONFIG_SND_SOC_SMDK_WM8580_PCM) += snd-soc-smdk-wm8580pcm.o
+obj-$(CONFIG_SND_SOC_SMDK_WM8994_PCM) += snd-soc-smdk-wm8994pcm.o
diff --git a/sound/soc/samsung/smdk_wm8994pcm.c b/sound/soc/samsung/smdk_wm8994pcm.c
new file mode 100644
index 0000000..756c228
--- /dev/null
+++ b/sound/soc/samsung/smdk_wm8994pcm.c
@@ -0,0 +1,175 @@
+/*
+ *  sound/soc/samsung/smdk_wm8994pcm.c
+ *
+ *  Copyright (c) 2011 Samsung Electronics Co. Ltd
+ *
+ *  This program is free software; you can redistribute  it and/or  modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ */
+#include <sound/soc.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+
+#include "../codecs/wm8994.h"
+#include "dma.h"
+#include "pcm.h"
+
+/*
+ * Board Settings:
+ *  o '1' means 'ON'
+ *  o '0' means 'OFF'
+ *  o 'X' means 'Don't care'
+ *
+ * SMDKC210, SMDKV310: CFG3- 1001, CFG5-1000, CFG7-111111
+ */
+
+/*
+ * Configure audio route as :-
+ * $ amixer sset 'DAC1' on,on
+ * $ amixer sset 'Right Headphone Mux' 'DAC'
+ * $ amixer sset 'Left Headphone Mux' 'DAC'
+ * $ amixer sset 'DAC1R Mixer AIF1.1' on
+ * $ amixer sset 'DAC1L Mixer AIF1.1' on
+ * $ amixer sset 'IN2L' on
+ * $ amixer sset 'IN2L PGA IN2LN' on
+ * $ amixer sset 'MIXINL IN2L' on
+ * $ amixer sset 'AIF1ADC1L Mixer ADC/DMIC' on
+ * $ amixer sset 'IN2R' on
+ * $ amixer sset 'IN2R PGA IN2RN' on
+ * $ amixer sset 'MIXINR IN2R' on
+ * $ amixer sset 'AIF1ADC1R Mixer ADC/DMIC' on
+ */
+
+/* SMDK has a 16.9344MHZ crystal attached to WM8994 */
+#define SMDK_WM8994_FREQ 16934400
+
+static int smdk_wm8994_pcm_hw_params(struct snd_pcm_substream *substream,
+			      struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	unsigned long mclk_freq;
+	int rfs, ret;
+
+	switch(params_rate(params)) {
+	case 8000:
+		rfs = 512;
+		break;
+	default:
+		printk(KERN_ERR "%s:%d Sampling Rate %u not supported!\n",
+		__func__, __LINE__, params_rate(params));
+		return -EINVAL;
+	}
+
+	mclk_freq = params_rate(params) * rfs;
+
+	/* Set the codec DAI configuration */
+	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_DSP_B
+				| SND_SOC_DAIFMT_IB_NF
+				| SND_SOC_DAIFMT_CBS_CFS);
+	if (ret < 0)
+		return ret;
+
+	/* Set the cpu DAI configuration */
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_DSP_B
+				| SND_SOC_DAIFMT_IB_NF
+				| SND_SOC_DAIFMT_CBS_CFS);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, WM8994_SYSCLK_FLL1,
+					mclk_freq, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_soc_dai_set_pll(codec_dai, WM8994_FLL1, WM8994_FLL_SRC_MCLK1,
+					SMDK_WM8994_FREQ, mclk_freq);
+	if (ret < 0)
+		return ret;
+
+	/* Set PCM source clock on CPU */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, S3C_PCM_CLKSRC_MUX,
+					mclk_freq, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* Set SCLK_DIV for making bclk */
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C_PCM_SCLK_PER_FS, rfs);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct snd_soc_ops smdk_wm8994_pcm_ops = {
+	.hw_params = smdk_wm8994_pcm_hw_params,
+};
+
+static struct snd_soc_dai_link smdk_dai[] = {
+	{
+		.name = "WM8994 PAIF PCM",
+		.stream_name = "Primary PCM",
+		.cpu_dai_name = "samsung-pcm.0",
+		.codec_dai_name = "wm8994-aif1",
+		.platform_name = "samsung-audio",
+		.codec_name = "wm8994-codec",
+		.ops = &smdk_wm8994_pcm_ops,
+	},
+};
+
+static struct snd_soc_card smdk_pcm = {
+	.name = "SMDK-PCM",
+	.dai_link = smdk_dai,
+	.num_links = 1,
+};
+
+static int __devinit snd_smdk_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+
+	smdk_pcm.dev = &pdev->dev;
+	ret = snd_soc_register_card(&smdk_pcm);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __devexit snd_smdk_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_card(&smdk_pcm);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+static struct platform_driver snd_smdk_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "samsung-smdk-pcm",
+	},
+	.probe = snd_smdk_probe,
+	.remove = __devexit_p(snd_smdk_remove),
+};
+
+static int __init smdk_audio_init(void)
+{
+	return platform_driver_register(&snd_smdk_driver);
+}
+
+module_init(smdk_audio_init);
+
+static void __exit smdk_audio_exit(void)
+{
+	platform_driver_unregister(&snd_smdk_driver);
+}
+
+module_exit(smdk_audio_exit);
+
+MODULE_AUTHOR("Sangbeom Kim, <sbkim73@samsung.com>");
+MODULE_DESCRIPTION("ALSA SoC SMDK WM8994 for PCM");
+MODULE_LICENSE("GPL");
-- 
1.7.1

[PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994

[Sangbeom Kim]

This patch add pcm audio configuration for SMDKV310 and SMDKC210.
Platform device and pcm clock initialization code is added.

Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
---
 arch/arm/mach-exynos4/clock.c                   |  169 ++++++++++++++++++++++-
 arch/arm/mach-exynos4/cpu.c                     |    5 +
 arch/arm/mach-exynos4/dev-audio.c               |    5 +
 arch/arm/mach-exynos4/include/mach/map.h        |    3 +
 arch/arm/mach-exynos4/include/mach/regs-audss.h |   25 ++++
 arch/arm/mach-exynos4/mach-smdkc210.c           |    7 +
 arch/arm/mach-exynos4/mach-smdkv310.c           |    7 +
 arch/arm/plat-samsung/include/plat/audio.h      |    2 +
 8 files changed, 221 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-exynos4/include/mach/regs-audss.h

diff --git a/arch/arm/mach-exynos4/clock.c b/arch/arm/mach-exynos4/clock.c
index 871f9d5..77cd81f 100644
--- a/arch/arm/mach-exynos4/clock.c
+++ b/arch/arm/mach-exynos4/clock.c
@@ -23,6 +23,7 @@
 
 #include <mach/map.h>
 #include <mach/regs-clock.h>
+#include <mach/regs-audss.h>
 #include <mach/sysmmu.h>
 
 static struct clk clk_sclk_hdmi27m = {
@@ -47,6 +48,31 @@ static struct clk clk_sclk_usbphy1 = {
 	.id		= -1,
 };
 
+static struct clk clk_sclk_xxti = {
+	.name		= "sclk_usbphy1",
+	.id		= -1,
+};
+
+static struct clk clk_sclk_xusbxti = {
+	.name		= "sclk_usbphy1",
+	.id		= -1,
+};
+
+static struct clk clk_audiocdclk0 = {
+	.name		= "audiocdclk",
+	.id		= 0,
+};
+
+static struct clk clk_audiocdclk1 = {
+	.name		= "audiocdclk",
+	.id		= 1,
+};
+
+static struct clk clk_audiocdclk2 = {
+	.name		= "audiocdclk",
+	.id		= 2,
+};
+
 static int exynos4_clksrc_mask_top_ctrl(struct clk *clk, int enable)
 {
 	return s5p_gatectrl(S5P_CLKSRC_MASK_TOP, clk, enable);
@@ -127,6 +153,16 @@ static int exynos4_clk_ip_perir_ctrl(struct clk *clk, int enable)
 	return s5p_gatectrl(S5P_CLKGATE_IP_PERIR, clk, enable);
 }
 
+static int exynos4_clksrc_mask_maudio_ctrl(struct clk *clk, int enable)
+{
+        return s5p_gatectrl(S5P_CLKSRC_MASK_MAUDIO, clk, enable);
+}
+
+static int exynos4_clk_audss_ctrl(struct clk *clk, int enable)
+{
+        return s5p_gatectrl(S5P_CLKGATE_AUDSS, clk, enable);
+}
+
 /* Core list of CMU_CPU side */
 
 static struct clksrc_clk clk_mout_apll = {
@@ -561,6 +597,21 @@ static struct clk init_clocks_off[] = {
 		.enable		= exynos4_clk_ip_peril_ctrl,
 		.ctrlbit	= (1 << 27),
 	}, {
+		.name		= "pcm",
+		.id		= 0,
+		.enable		= exynos4_clk_audss_ctrl,
+		.ctrlbit	= S5P_AUDSS_CLKGATE_PCMSPECIAL,
+	}, {
+		.name		= "pcm",
+		.id		= 1,
+		.enable		= exynos4_clk_ip_peril_ctrl,
+		.ctrlbit	= (1 << 22),
+	}, {
+		.name		= "pcm",
+		.id		= 2,
+		.enable		= exynos4_clk_ip_peril_ctrl,
+		.ctrlbit	= (1 << 23),
+        }, {
 		.name		= "fimg2d",
 		.id		= -1,
 		.enable		= exynos4_clk_ip_image_ctrl,
@@ -686,6 +737,93 @@ static struct clk init_clocks_off[] = {
 	}
 };
 
+static struct clk *clkset_sclk_audio0_list[] = {
+	[0] = &clk_audiocdclk0,
+	[1] = NULL,
+	[2] = &clk_sclk_hdmi27m,
+	[3] = &clk_sclk_usbphy0,
+	[4] = &clk_sclk_xxti,
+	[5] = &clk_sclk_xusbxti,
+	[6] = &clk_mout_mpll.clk,
+	[7] = &clk_mout_epll.clk,
+	[8] = &clk_sclk_vpll.clk,
+};
+
+static struct clksrc_sources clkset_sclk_audio0 = {
+	.sources	= clkset_sclk_audio0_list,
+	.nr_sources	= ARRAY_SIZE(clkset_sclk_audio0_list),
+};
+
+static struct clksrc_clk clk_sclk_audio0 = {
+	.clk	= {
+		.name		= "audio-bus",
+		.id		= 0,
+		.enable		= exynos4_clksrc_mask_maudio_ctrl,
+		.ctrlbit	= (1 << 0),
+	},
+	.sources = &clkset_sclk_audio0,
+	.reg_src = { .reg = S5P_CLKSRC_MAUDIO, .shift = 0, .size = 4 },
+	.reg_div = { .reg = S5P_CLKDIV_MAUDIO, .shift = 0, .size = 4 },
+};
+
+static struct clk *clkset_sclk_audio1_list[] = {
+	[0] = &clk_audiocdclk1,
+	[1] = NULL,
+	[2] = &clk_sclk_hdmi27m,
+	[3] = &clk_sclk_usbphy0,
+	[4] = &clk_sclk_xxti,
+	[5] = &clk_sclk_xusbxti,
+	[6] = &clk_mout_mpll.clk,
+	[7] = &clk_mout_epll.clk,
+	[8] = &clk_sclk_vpll.clk,
+};
+
+static struct clksrc_sources clkset_sclk_audio1 = {
+	.sources	= clkset_sclk_audio1_list,
+	.nr_sources	= ARRAY_SIZE(clkset_sclk_audio1_list),
+};
+
+static struct clksrc_clk clk_sclk_audio1 = {
+	.clk	= {
+		.name		= "audio-bus",
+		.id		= 1,
+		.enable		= exynos4_clksrc_mask_peril1_ctrl,
+		.ctrlbit	= (1 << 0),
+        },
+	.sources = &clkset_sclk_audio1,
+	.reg_src = { .reg = S5P_CLKSRC_PERIL1, .shift = 0, .size = 4 },
+	.reg_div = { .reg = S5P_CLKDIV_PERIL4, .shift = 4, .size = 8 },
+};
+
+static struct clk *clkset_sclk_audio2_list[] = {
+	[0] = &clk_audiocdclk2,
+	[1] = NULL,
+	[2] = &clk_sclk_hdmi27m,
+	[3] = &clk_sclk_usbphy0,
+	[4] = &clk_sclk_xxti,
+	[5] = &clk_sclk_xusbxti,
+	[6] = &clk_mout_mpll.clk,
+	[7] = &clk_mout_epll.clk,
+	[8] = &clk_sclk_vpll.clk,
+};
+
+static struct clksrc_sources clkset_sclk_audio2 = {
+	.sources        = clkset_sclk_audio2_list,
+	.nr_sources     = ARRAY_SIZE(clkset_sclk_audio2_list),
+};
+
+static struct clksrc_clk clk_sclk_audio2 = {
+	.clk	= {
+		.name		= "audio-bus",
+		.id		= 2,
+		.enable		= exynos4_clksrc_mask_peril1_ctrl,
+		.ctrlbit	= (1 << 4),
+	},
+	.sources = &clkset_sclk_audio2,
+	.reg_src = { .reg = S5P_CLKSRC_PERIL1, .shift = 4, .size = 4 },
+	.reg_div = { .reg = S5P_CLKDIV_PERIL4, .shift = 16, .size = 4 },
+};
+
 static struct clk init_clocks[] = {
 	{
 		.name		= "uart",
@@ -1079,7 +1217,28 @@ static struct clksrc_clk clksrcs[] = {
 			.ctrlbit	= (1 << 16),
 		},
 		.reg_div = { .reg = S5P_CLKDIV_FSYS3, .shift = 8, .size = 8 },
-	}
+	}, {
+		.clk		= {
+			.name		= "sclk_pcm",
+			.id		= 0,
+			.parent		= &clk_sclk_audio0.clk,
+                },
+                .reg_div = { .reg = S5P_CLKDIV_MAUDIO, .shift = 4, .size = 8 },
+        }, {
+                .clk            = {
+                        .name           = "sclk_pcm",
+                        .id             = 1,
+                        .parent         = &clk_sclk_audio1.clk,
+                },
+                .reg_div = { .reg = S5P_CLKDIV_PERIL4, .shift = 4, .size = 8 },
+        }, {
+                .clk            = {
+                        .name           = "sclk_pcm",
+                        .id             = 2,
+                        .parent         = &clk_sclk_audio2.clk,
+                },
+                .reg_div = { .reg = S5P_CLKDIV_PERIL4, .shift = 20, .size = 8 },
+        },
 };
 
 /* Clock initialization code */
@@ -1112,6 +1271,9 @@ static struct clksrc_clk *sysclks[] = {
 	&clk_dout_mmc2,
 	&clk_dout_mmc3,
 	&clk_dout_mmc4,
+	&clk_sclk_audio0,
+	&clk_sclk_audio1,
+	&clk_sclk_audio2,
 };
 
 static int xtal_rate;
@@ -1191,10 +1353,13 @@ void __init_or_cpufreq exynos4_setup_clocks(void)
 
 	for (ptr = 0; ptr < ARRAY_SIZE(clksrcs); ptr++)
 		s3c_set_clksrc(&clksrcs[ptr], true);
+
+	clk_audiocdclk0.rate = PCM_EXTCLK0;
+	clk_set_parent(&clk_sclk_audio0.clk, &clk_audiocdclk0);
 }
 
 static struct clk *clks[] __initdata = {
-	/* Nothing here yet */
+	&clk_audiocdclk0,
 };
 
 void __init exynos4_register_clocks(void)
diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 7930113..620b41f 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -97,6 +97,11 @@ static struct map_desc exynos4_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(EXYNOS4_PA_SROMC),
 		.length		= SZ_4K,
 		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_AUDSS,
+		.pfn		= __phys_to_pfn(EXYNOS4_PA_AUDSS),
+		.length		= SZ_1K,
+		.type		= MT_DEVICE,
 	},
 };
 
diff --git a/arch/arm/mach-exynos4/dev-audio.c b/arch/arm/mach-exynos4/dev-audio.c
index 1eed5f9..383a4aa 100644
--- a/arch/arm/mach-exynos4/dev-audio.c
+++ b/arch/arm/mach-exynos4/dev-audio.c
@@ -187,6 +187,11 @@ static int exynos4_pcm_cfg_gpio(struct platform_device *pdev)
 
 static struct s3c_audio_pdata s3c_pcm_pdata = {
 	.cfg_gpio = exynos4_pcm_cfg_gpio,
+	.type = {
+		.i2s = {
+			.quirks = QUIRK_NO_DIV,
+		},
+	},
 };
 
 static struct resource exynos4_pcm0_resource[] = {
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 6330b73..063854e 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -30,6 +30,8 @@
 #define EXYNOS4_PA_FIMC2		0x11820000
 #define EXYNOS4_PA_FIMC3		0x11830000
 
+#define EXYNOS4_PA_AUDSS		0x03810000
+
 #define EXYNOS4_PA_I2S0			0x03830000
 #define EXYNOS4_PA_I2S1			0xE3100000
 #define EXYNOS4_PA_I2S2			0xE2A00000
@@ -143,6 +145,7 @@
 #define S5P_PA_SROMC			EXYNOS4_PA_SROMC
 #define S5P_PA_SYSCON			EXYNOS4_PA_SYSCON
 #define S5P_PA_TIMER			EXYNOS4_PA_TIMER
+#define S5P_PA_AUDSS			EXYNOS4_PA_AUDSS
 
 #define SAMSUNG_PA_KEYPAD		EXYNOS4_PA_KEYPAD
 
diff --git a/arch/arm/mach-exynos4/include/mach/regs-audss.h b/arch/arm/mach-exynos4/include/mach/regs-audss.h
new file mode 100644
index 0000000..23c1d3a
--- /dev/null
+++ b/arch/arm/mach-exynos4/include/mach/regs-audss.h
@@ -0,0 +1,25 @@
+/* arch/arm/mach-exynos4/include/mach/regs-audss.h
+ *
+ * Copyright (c) 2011 Samsung Electronics
+ *		http://www.samsung.com
+ *
+ * Exynos4 Audio SubSystem clock register definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_REGS_AUDSS_H
+#define __PLAT_REGS_AUDSS_H __FILE__
+
+#define S5P_AUDSSREG(x)		(S5P_VA_AUDSS + (x))
+
+#define S5P_CLKGATE_AUDSS	S5P_AUDSSREG(0x8)
+
+#define PCM_EXTCLK0		16934400	
+
+/* IP Clock Gate 0 Registers */
+#define S5P_AUDSS_CLKGATE_PCMSPECIAL	(1<<5)
+
+#endif /* _PLAT_REGS_AUDSS_H */
diff --git a/arch/arm/mach-exynos4/mach-smdkc210.c b/arch/arm/mach-exynos4/mach-smdkc210.c
index e645f7a..ac53e49 100644
--- a/arch/arm/mach-exynos4/mach-smdkc210.c
+++ b/arch/arm/mach-exynos4/mach-smdkc210.c
@@ -142,6 +142,11 @@ static struct platform_device smdkc210_smsc911x = {
 	},
 };
 
+static struct platform_device smdkc210_pcm_device = {
+	.name		= "samsung-smdk-pcm",
+	.id		= -1,
+};
+
 static struct i2c_board_info i2c_devs1[] __initdata = {
 	{I2C_BOARD_INFO("wm8994", 0x1a),},
 };
@@ -156,6 +161,7 @@ static struct platform_device *smdkc210_devices[] __initdata = {
 	&s3c_device_wdt,
 	&exynos4_device_ac97,
 	&exynos4_device_i2s0,
+	&exynos4_device_pcm0,
 	&exynos4_device_pd[PD_MFC],
 	&exynos4_device_pd[PD_G3D],
 	&exynos4_device_pd[PD_LCD0],
@@ -166,6 +172,7 @@ static struct platform_device *smdkc210_devices[] __initdata = {
 	&exynos4_device_sysmmu,
 	&samsung_asoc_dma,
 	&smdkc210_smsc911x,
+	&smdkc210_pcm_device,
 };
 
 static void __init smdkc210_smsc911x_init(void)
diff --git a/arch/arm/mach-exynos4/mach-smdkv310.c b/arch/arm/mach-exynos4/mach-smdkv310.c
index 1526764..7e3ee6c 100644
--- a/arch/arm/mach-exynos4/mach-smdkv310.c
+++ b/arch/arm/mach-exynos4/mach-smdkv310.c
@@ -163,6 +163,11 @@ static struct samsung_keypad_platdata smdkv310_keypad_data __initdata = {
 	.cols		= 8,
 };
 
+static struct platform_device smdkv310_pcm_device = {
+	.name		= "samsung-smdk-pcm",
+	.id		= -1,
+};
+
 static struct i2c_board_info i2c_devs1[] __initdata = {
 	{I2C_BOARD_INFO("wm8994", 0x1a),},
 };
@@ -186,8 +191,10 @@ static struct platform_device *smdkv310_devices[] __initdata = {
 	&exynos4_device_pd[PD_TV],
 	&exynos4_device_pd[PD_GPS],
 	&exynos4_device_sysmmu,
+	&exynos4_device_pcm0,
 	&samsung_asoc_dma,
 	&smdkv310_smsc911x,
+	&smdkv310_pcm_device,
 };
 
 static void __init smdkv310_smsc911x_init(void)
diff --git a/arch/arm/plat-samsung/include/plat/audio.h b/arch/arm/plat-samsung/include/plat/audio.h
index a0826ed..1310547 100644
--- a/arch/arm/plat-samsung/include/plat/audio.h
+++ b/arch/arm/plat-samsung/include/plat/audio.h
@@ -36,6 +36,8 @@ struct samsung_i2s {
  */
 #define QUIRK_NO_MUXPSR		(1 << 2)
 #define QUIRK_NEED_RSTCLR	(1 << 3)
+/* If the PCM block has no internal prescalar or MUX */
+#define QUIRK_NO_DIV		(1 << 4)
 	/* Quirks of the I2S controller */
 	u32 quirks;
 
-- 
1.7.1

Re: [PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994

[Jassi Brar]

On Mon, Apr 25, 2011 at 12:24 PM, Sangbeom Kim <sbkim73@samsung.com> wrote:

> --- a/arch/arm/plat-samsung/include/plat/audio.h
> +++ b/arch/arm/plat-samsung/include/plat/audio.h
> @@ -36,6 +36,8 @@ struct samsung_i2s {
>  */
>  #define QUIRK_NO_MUXPSR                (1 << 2)
>  #define QUIRK_NEED_RSTCLR      (1 << 3)
> +/* If the PCM block has no internal prescalar or MUX */
> +#define QUIRK_NO_DIV           (1 << 4)
>        /* Quirks of the I2S controller */
>        u32 quirks;

I think the I2S and PCM configuration don't share enough.
So maybe a separate 'struct samsung_pcm' is warranted here.
Also, if new SoC PCM blocks are going to _not_ have the divider
may be it's a good idea to make that as 'norm' and having divider
as a 'quirk' ?  That way, new code wouldn't have to bother adding
the quirk every time.

-j

RE: [alsa-devel] [PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994

[Sangbeom Kim]

Hi Jassi,

Thanks for your advisor.
Currently, Only Exynos4210 pcm block is different.
And There is no rule to design in Future AP.
I will discuss with ap design team and try to
change pcm block like a previous one for the future AP.
Because, I think that it is more desirable to include
SCLK divider in the pcm block like old Samsung APs.
So, In the exynos4 case, I want to handle it by quirk.
What do you think about my opinion?

SB Kim


On Tuesday, April 26, 2011 1:10 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> To: Sangbeom Kim
> Cc: kgene.kim@samsung.com; alsa-devel@alsa-project.org; linux-samsung-
> soc@vger.kernel.org; broonie@opensource.wolfsonmicro.com;
> lrg@slimlogic.co.uk
> Subject: Re: [alsa-devel] [PATCH 1/2] ARM: EXYNOS4: Add PCM audio support
> for WM8994
> 
> On Mon, Apr 25, 2011 at 12:24 PM, Sangbeom Kim <sbkim73@samsung.com> wrote:
> 
> > --- a/arch/arm/plat-samsung/include/plat/audio.h
> > +++ b/arch/arm/plat-samsung/include/plat/audio.h
> > @@ -36,6 +36,8 @@ struct samsung_i2s {
> >  */
> >  #define QUIRK_NO_MUXPSR                (1 << 2)
> >  #define QUIRK_NEED_RSTCLR      (1 << 3)
> > +/* If the PCM block has no internal prescalar or MUX */
> > +#define QUIRK_NO_DIV           (1 << 4)
> >        /* Quirks of the I2S controller */
> >        u32 quirks;
> 
> I think the I2S and PCM configuration don't share enough.
> So maybe a separate 'struct samsung_pcm' is warranted here.
> Also, if new SoC PCM blocks are going to _not_ have the divider
> may be it's a good idea to make that as 'norm' and having divider
> as a 'quirk' ?  That way, new code wouldn't have to bother adding
> the quirk every time.
> 
> -j
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994

[Jassi Brar]

On Tue, Apr 26, 2011 at 7:36 AM, Sangbeom Kim <sbkim73@samsung.com> wrote:
> Hi Jassi,
>
> Thanks for your advisor.
> Currently, Only Exynos4210 pcm block is different.
> And There is no rule to design in Future AP.
> I will discuss with ap design team and try to
> change pcm block like a previous one for the future AP.
> Because, I think that it is more desirable to include
> SCLK divider in the pcm block like old Samsung APs.
> So, In the exynos4 case, I want to handle it by quirk.
> What do you think about my opinion?

Ok. If the block is not going to change for sure, maybe
handling by quirk is ok.

Re: [alsa-devel] (no subject)

[Liam Girdwood]

On Mon, 2011-04-25 at 15:54 +0900, Sangbeom Kim wrote:
> [PATCH 0/2] Add WM8994 PCM Machine driver for Exynos4
> 
> Hi,
> 
> This is patchset for WM8994 pcm machine driver that supports
> PCM audio on SMDKV310, SMDKC210 and test is done.
> 
> Based on these patches WM8994 pcm machine driver supports followings:
>  o Playback supports 8kHz sampling rates.
>  o Capture supports 8kHz sampling rates.
> 
> This patchset contains followings 
> 
>  o To Kukjin Kim and Ben Dooks,
> [PATCH 1/2] ARM: EXYNOS4: Add PCM audio support for WM8994
> 
>  o To Jassi Brar, Mark Brown and Liam Girdwood,
> [PATCH 2/2] ASoC: SAMSUNG: Add WM8580 PCM Machine driver 
> 
> Best Regards,
> SB Kim (Sangbeom Kim)
> 

Both

Acked-by: Liam Girdwood <lrg@ti.com>

Re: [alsa-devel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Ben Dooks]

On 20/12/2019 16:40, Dmitry Osipenko wrote:
> 20.12.2019 18:25, Ben Dooks пишет:
>> On 20/12/2019 15:02, Dmitry Osipenko wrote:
>>> 20.12.2019 17:56, Ben Dooks пишет:
>>>> On 20/12/2019 14:43, Dmitry Osipenko wrote:
>>>>> 20.12.2019 16:57, Jon Hunter пишет:
>>>>>>
>>>>>> On 20/12/2019 11:38, Ben Dooks wrote:
>>>>>>> On 20/12/2019 11:30, Jon Hunter wrote:
>>>>>>>>
>>>>>>>> On 25/11/2019 17:28, Dmitry Osipenko wrote:
>>>>>>>>> 25.11.2019 20:22, Dmitry Osipenko пишет:
>>>>>>>>>> 25.11.2019 13:37, Ben Dooks пишет:
>>>>>>>>>>> On 23/11/2019 21:09, Dmitry Osipenko wrote:
>>>>>>>>>>>> 18.10.2019 18:48, Ben Dooks пишет:
>>>>>>>>>>>>> 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: drop debug printing to dev_dbg]
>>>>>>>>>>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> 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 | 25
>>>>>>>>>>>>> ++++++++++++++++++++-----
>>>>>>>>>>>>>       1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>>>>>>>>>>>> b/sound/soc/tegra/tegra30_i2s.c
>>>>>>>>>>>>> index 73f0dddeaef3..063f34c882af 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;
>>>>>>>>>>>>> @@ -306,14 +317,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,
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> This patch breaks audio on Tegra30. I don't see errors
>>>>>>>>>>>> anywhere, but
>>>>>>>>>>>> there is no audio and reverting this patch helps. Please fix it.
>>>>>>>>>>>
>>>>>>>>>>> What is the failure mode? I can try and take a look at this some
>>>>>>>>>>> time
>>>>>>>>>>> this week, but I am not sure if I have any boards with an actual
>>>>>>>>>>> useful
>>>>>>>>>>> audio output?
>>>>>>>>>>
>>>>>>>>>> The failure mode is that there no sound. I also noticed that video
>>>>>>>>>> playback stutters a lot if movie file has audio track, seems
>>>>>>>>>> something
>>>>>>>>>> times out during of the audio playback. For now I don't have any
>>>>>>>>>> more info.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Oh, I didn't say how to reproduce it.. for example simply playing
>>>>>>>>> big_buck_bunny_720p_h264.mov in MPV has the audio problem.
>>>>>>>>>
>>>>>>>>> https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Given that the audio drivers uses regmap, it could be good to
>>>>>>>> dump the
>>>>>>>> I2S/AHUB registers while the clip if playing with and without this
>>>>>>>> patch
>>>>>>>> to see the differences. I am curious if the audio is now being
>>>>>>>> played as
>>>>>>>> 24 or 32-bit instead of 16-bit now these are available.
>>>>>>>>
>>>>>>>> You could also dump the hw_params to see the format while playing as
>>>>>>>> well ...
>>>>>>>>
>>>>>>>> $ /proc/asound/<scard-name>/pcm0p/sub0/hw_params
>>>>>>>
>>>>>>> I suppose it is also possible that the codec isn't properly doing >16
>>>>>>> bits and the fact we now offer 24 and 32 could be an issue. I've not
>>>>>>> got anything with an audio output on it that would be easy to test.
>>>>>>
>>>>>> I thought I had tested on a Jetson TK1 (Tegra124) but it was sometime
>>>>>> back. However, admittedly I may have only done simple 16-bit testing
>>>>>> with speaker-test.
>>>>>>
>>>>>> We do verify that all soundcards are detected and registered as
>>>>>> expected
>>>>>> during daily testing, but at the moment we don't have anything that
>>>>>> verifies actual playback.
>>>>>
>>>>> Please take a look at the attached logs.
>>>>
>>>> I wonder if we are falling into FIFO configuration issues with the
>>>> non-16 bit case.
>>>>
>>>> Can you try adding the following two patches?
>>>
>>> It is much better now! There is no horrible noise and no stuttering, but
>>> audio still has a "robotic" effect, like freq isn't correct.
>>
>> I wonder if there's an issue with FIFO stoking? I should also possibly
>> add the correctly stop FIFOs patch as well.
> 
> I'll be happy to try more patches.
> 
> Meanwhile sound on v5.5+ is broken and thus the incomplete patches
> should be reverted.

Have you checked if just removing the 24/32 bits from .formats in
the dai template and see if the problem continues? I will try and
see if I can find the code that does the fifo level checking and
see if the problem is in the FIFO fill or something else in the
audio hub setup.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

20.12.2019 20:06, Ben Dooks пишет:
> On 20/12/2019 16:40, Dmitry Osipenko wrote:
>> 20.12.2019 18:25, Ben Dooks пишет:
>>> On 20/12/2019 15:02, Dmitry Osipenko wrote:
>>>> 20.12.2019 17:56, Ben Dooks пишет:
>>>>> On 20/12/2019 14:43, Dmitry Osipenko wrote:
>>>>>> 20.12.2019 16:57, Jon Hunter пишет:
>>>>>>>
>>>>>>> On 20/12/2019 11:38, Ben Dooks wrote:
>>>>>>>> On 20/12/2019 11:30, Jon Hunter wrote:
>>>>>>>>>
>>>>>>>>> On 25/11/2019 17:28, Dmitry Osipenko wrote:
>>>>>>>>>> 25.11.2019 20:22, Dmitry Osipenko пишет:
>>>>>>>>>>> 25.11.2019 13:37, Ben Dooks пишет:
>>>>>>>>>>>> On 23/11/2019 21:09, Dmitry Osipenko wrote:
>>>>>>>>>>>>> 18.10.2019 18:48, Ben Dooks пишет:
>>>>>>>>>>>>>> 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: drop debug printing to dev_dbg]
>>>>>>>>>>>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>> 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 | 25
>>>>>>>>>>>>>> ++++++++++++++++++++-----
>>>>>>>>>>>>>>       1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/sound/soc/tegra/tegra30_i2s.c
>>>>>>>>>>>>>> b/sound/soc/tegra/tegra30_i2s.c
>>>>>>>>>>>>>> index 73f0dddeaef3..063f34c882af 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;
>>>>>>>>>>>>>> @@ -306,14 +317,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,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>
>>>>>>>>>>>>> This patch breaks audio on Tegra30. I don't see errors
>>>>>>>>>>>>> anywhere, but
>>>>>>>>>>>>> there is no audio and reverting this patch helps. Please
>>>>>>>>>>>>> fix it.
>>>>>>>>>>>>
>>>>>>>>>>>> What is the failure mode? I can try and take a look at this
>>>>>>>>>>>> some
>>>>>>>>>>>> time
>>>>>>>>>>>> this week, but I am not sure if I have any boards with an
>>>>>>>>>>>> actual
>>>>>>>>>>>> useful
>>>>>>>>>>>> audio output?
>>>>>>>>>>>
>>>>>>>>>>> The failure mode is that there no sound. I also noticed that
>>>>>>>>>>> video
>>>>>>>>>>> playback stutters a lot if movie file has audio track, seems
>>>>>>>>>>> something
>>>>>>>>>>> times out during of the audio playback. For now I don't have any
>>>>>>>>>>> more info.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Oh, I didn't say how to reproduce it.. for example simply playing
>>>>>>>>>> big_buck_bunny_720p_h264.mov in MPV has the audio problem.
>>>>>>>>>>
>>>>>>>>>> https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Given that the audio drivers uses regmap, it could be good to
>>>>>>>>> dump the
>>>>>>>>> I2S/AHUB registers while the clip if playing with and without this
>>>>>>>>> patch
>>>>>>>>> to see the differences. I am curious if the audio is now being
>>>>>>>>> played as
>>>>>>>>> 24 or 32-bit instead of 16-bit now these are available.
>>>>>>>>>
>>>>>>>>> You could also dump the hw_params to see the format while
>>>>>>>>> playing as
>>>>>>>>> well ...
>>>>>>>>>
>>>>>>>>> $ /proc/asound/<scard-name>/pcm0p/sub0/hw_params
>>>>>>>>
>>>>>>>> I suppose it is also possible that the codec isn't properly
>>>>>>>> doing >16
>>>>>>>> bits and the fact we now offer 24 and 32 could be an issue. I've
>>>>>>>> not
>>>>>>>> got anything with an audio output on it that would be easy to test.
>>>>>>>
>>>>>>> I thought I had tested on a Jetson TK1 (Tegra124) but it was
>>>>>>> sometime
>>>>>>> back. However, admittedly I may have only done simple 16-bit testing
>>>>>>> with speaker-test.
>>>>>>>
>>>>>>> We do verify that all soundcards are detected and registered as
>>>>>>> expected
>>>>>>> during daily testing, but at the moment we don't have anything that
>>>>>>> verifies actual playback.
>>>>>>
>>>>>> Please take a look at the attached logs.
>>>>>
>>>>> I wonder if we are falling into FIFO configuration issues with the
>>>>> non-16 bit case.
>>>>>
>>>>> Can you try adding the following two patches?
>>>>
>>>> It is much better now! There is no horrible noise and no stuttering,
>>>> but
>>>> audio still has a "robotic" effect, like freq isn't correct.
>>>
>>> I wonder if there's an issue with FIFO stoking? I should also possibly
>>> add the correctly stop FIFOs patch as well.
>>
>> I'll be happy to try more patches.
>>
>> Meanwhile sound on v5.5+ is broken and thus the incomplete patches
>> should be reverted.
> 
> Have you checked if just removing the 24/32 bits from .formats in
> the dai template and see if the problem continues? 

It works.

> I will try and
> see if I can find the code that does the fifo level checking and
> see if the problem is in the FIFO fill or something else in the
> audio hub setup.

Ok
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Ben Dooks]

[snip]

I've just gone through testing.

Some simple data tests show 16 and 32-bits work.

The 24 bit case seems to be weird, it looks like the 24-bit expects
24 bit samples in 32 bit words. I can't see any packing options to
do 24 bit in 24 bit, so we may have to remove 24 bit sample support
(which is a shame)

My preference is to remove the 24-bit support and keep the 32 bit in.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

05.01.2020 03:04, Ben Dooks пишет:
> [snip]
> 
> I've just gone through testing.
> 
> Some simple data tests show 16 and 32-bits work.
> 
> The 24 bit case seems to be weird, it looks like the 24-bit expects
> 24 bit samples in 32 bit words. I can't see any packing options to
> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
> (which is a shame)
> 
> My preference is to remove the 24-bit support and keep the 32 bit in.
> 

Interesting.. Jon, could you please confirm that 24bit format isn't
usable on T30?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Ben Dooks]

On 2020-01-05 01:48, Dmitry Osipenko wrote:
> 05.01.2020 03:04, Ben Dooks пишет:
>> [snip]
>> 
>> I've just gone through testing.
>> 
>> Some simple data tests show 16 and 32-bits work.
>> 
>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>> 24 bit samples in 32 bit words. I can't see any packing options to
>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>> (which is a shame)
>> 
>> My preference is to remove the 24-bit support and keep the 32 bit in.
>> 
> 
> Interesting.. Jon, could you please confirm that 24bit format isn't
> usable on T30?

If there is an option of 24 packed into 32, then I think that would 
work.

I can try testing that with raw data on Monday.

-- 
Ben

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Ben Dooks]

On 05/01/2020 10:53, Ben Dooks wrote:
> 
> 
> On 2020-01-05 01:48, Dmitry Osipenko wrote:
>> 05.01.2020 03:04, Ben Dooks пишет:
>>> [snip]
>>>
>>> I've just gone through testing.
>>>
>>> Some simple data tests show 16 and 32-bits work.
>>>
>>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>>> 24 bit samples in 32 bit words. I can't see any packing options to
>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>>> (which is a shame)
>>>
>>> My preference is to remove the 24-bit support and keep the 32 bit in.
>>>
>>
>> Interesting.. Jon, could you please confirm that 24bit format isn't
>> usable on T30?
> 
> If there is an option of 24 packed into 32, then I think that would work.
> 
> I can try testing that with raw data on Monday.

I need to check some things, I assumed 24 was 24 packed bits, it looks
like the default is 24 in 32 bits so we may be ok. However I need to
re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).

I'll follow up later,



-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

06.01.2020 22:00, Ben Dooks пишет:
> On 05/01/2020 10:53, Ben Dooks wrote:
>>
>>
>> On 2020-01-05 01:48, Dmitry Osipenko wrote:
>>> 05.01.2020 03:04, Ben Dooks пишет:
>>>> [snip]
>>>>
>>>> I've just gone through testing.
>>>>
>>>> Some simple data tests show 16 and 32-bits work.
>>>>
>>>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>>>> 24 bit samples in 32 bit words. I can't see any packing options to
>>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>>>> (which is a shame)
>>>>
>>>> My preference is to remove the 24-bit support and keep the 32 bit in.
>>>>
>>>
>>> Interesting.. Jon, could you please confirm that 24bit format isn't
>>> usable on T30?
>>
>> If there is an option of 24 packed into 32, then I think that would work.
>>
>> I can try testing that with raw data on Monday.
> 
> I need to check some things, I assumed 24 was 24 packed bits, it looks
> like the default is 24 in 32 bits so we may be ok. However I need to
> re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).
> 
> I'll follow up later,

Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
looked through the TRM doc and got impression that AHUB could re-pack
data stream into something that codec supports, but maybe it's a wrong
impression.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Ben Dooks]

On 07/01/2020 01:39, Dmitry Osipenko wrote:
> 06.01.2020 22:00, Ben Dooks пишет:
>> On 05/01/2020 10:53, Ben Dooks wrote:
>>>
>>>
>>> On 2020-01-05 01:48, Dmitry Osipenko wrote:
>>>> 05.01.2020 03:04, Ben Dooks пишет:
>>>>> [snip]
>>>>>
>>>>> I've just gone through testing.
>>>>>
>>>>> Some simple data tests show 16 and 32-bits work.
>>>>>
>>>>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>>>>> 24 bit samples in 32 bit words. I can't see any packing options to
>>>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>>>>> (which is a shame)
>>>>>
>>>>> My preference is to remove the 24-bit support and keep the 32 bit in.
>>>>>
>>>>
>>>> Interesting.. Jon, could you please confirm that 24bit format isn't
>>>> usable on T30?
>>>
>>> If there is an option of 24 packed into 32, then I think that would work.
>>>
>>> I can try testing that with raw data on Monday.
>>
>> I need to check some things, I assumed 24 was 24 packed bits, it looks
>> like the default is 24 in 32 bits so we may be ok. However I need to
>> re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).
>>
>> I'll follow up later,
> 
> Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
> looked through the TRM doc and got impression that AHUB could re-pack
> data stream into something that codec supports, but maybe it's a wrong
> impression.
> _________________________________

I did a quick test with the following:

  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
  sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
  sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5

The 16 and 32 work fine, the 24 is showing a playback output freq
of 440Hz instead of 500Hz... this suggests the clock is off, or there
is something else weird going on...

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Jon Hunter]

On 23/01/2020 19:38, Ben Dooks wrote:
> On 07/01/2020 01:39, Dmitry Osipenko wrote:
>> 06.01.2020 22:00, Ben Dooks пишет:
>>> On 05/01/2020 10:53, Ben Dooks wrote:
>>>>
>>>>
>>>> On 2020-01-05 01:48, Dmitry Osipenko wrote:
>>>>> 05.01.2020 03:04, Ben Dooks пишет:
>>>>>> [snip]
>>>>>>
>>>>>> I've just gone through testing.
>>>>>>
>>>>>> Some simple data tests show 16 and 32-bits work.
>>>>>>
>>>>>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>>>>>> 24 bit samples in 32 bit words. I can't see any packing options to
>>>>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>>>>>> (which is a shame)
>>>>>>
>>>>>> My preference is to remove the 24-bit support and keep the 32 bit in.
>>>>>>
>>>>>
>>>>> Interesting.. Jon, could you please confirm that 24bit format isn't
>>>>> usable on T30?
>>>>
>>>> If there is an option of 24 packed into 32, then I think that would
>>>> work.
>>>>
>>>> I can try testing that with raw data on Monday.
>>>
>>> I need to check some things, I assumed 24 was 24 packed bits, it looks
>>> like the default is 24 in 32 bits so we may be ok. However I need to
>>> re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).
>>>
>>> I'll follow up later,
>>
>> Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
>> looked through the TRM doc and got impression that AHUB could re-pack
>> data stream into something that codec supports, but maybe it's a wrong
>> impression.
>> _________________________________
> 
> I did a quick test with the following:
> 
>  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
>  sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
>  sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
> 
> The 16 and 32 work fine, the 24 is showing a playback output freq
> of 440Hz instead of 500Hz... this suggests the clock is off, or there
> is something else weird going on...

I was looking at using sox to create such as file, but the above command
generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1
supports S24_LE but does not support S24_3LE and so I cannot test this.
Anyway, we really need to test S24_LE and not S24_3LE because this is
the problem that Dmitry is having.

Ben is S24_3LE what you really need to support?

Dmitry, does the following fix your problem?

diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
index dbed3c5408e7..92845c4b63f4 100644
--- a/sound/soc/tegra/tegra30_i2s.c
+++ b/sound/soc/tegra/tegra30_i2s.c
@@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
snd_pcm_substream *substream,
                audio_bits = TEGRA30_AUDIOCIF_BITS_16;
                sample_size = 16;
                break;
-       case SNDRV_PCM_FORMAT_S24_LE:
+       case SNDRV_PCM_FORMAT_S24_3LE:
                val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
                audio_bits = TEGRA30_AUDIOCIF_BITS_24;
                sample_size = 24;
@@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },
        .capture = {
@@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
tegra30_i2s_dai_template = {
                .channels_max = 2,
                .rates = SNDRV_PCM_RATE_8000_96000,
                .formats = SNDRV_PCM_FMTBIT_S32_LE |
-                          SNDRV_PCM_FMTBIT_S24_LE |
+                          SNDRV_PCM_FMTBIT_S24_3LE |
                           SNDRV_PCM_FMTBIT_S16_LE,
        },
        .ops = &tegra30_i2s_dai_ops,

Jon

-- 
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

24.01.2020 19:50, Jon Hunter пишет:
> 
> On 23/01/2020 19:38, Ben Dooks wrote:
>> On 07/01/2020 01:39, Dmitry Osipenko wrote:
>>> 06.01.2020 22:00, Ben Dooks пишет:
>>>> On 05/01/2020 10:53, Ben Dooks wrote:
>>>>>
>>>>>
>>>>> On 2020-01-05 01:48, Dmitry Osipenko wrote:
>>>>>> 05.01.2020 03:04, Ben Dooks пишет:
>>>>>>> [snip]
>>>>>>>
>>>>>>> I've just gone through testing.
>>>>>>>
>>>>>>> Some simple data tests show 16 and 32-bits work.
>>>>>>>
>>>>>>> The 24 bit case seems to be weird, it looks like the 24-bit expects
>>>>>>> 24 bit samples in 32 bit words. I can't see any packing options to
>>>>>>> do 24 bit in 24 bit, so we may have to remove 24 bit sample support
>>>>>>> (which is a shame)
>>>>>>>
>>>>>>> My preference is to remove the 24-bit support and keep the 32 bit in.
>>>>>>>
>>>>>>
>>>>>> Interesting.. Jon, could you please confirm that 24bit format isn't
>>>>>> usable on T30?
>>>>>
>>>>> If there is an option of 24 packed into 32, then I think that would
>>>>> work.
>>>>>
>>>>> I can try testing that with raw data on Monday.
>>>>
>>>> I need to check some things, I assumed 24 was 24 packed bits, it looks
>>>> like the default is 24 in 32 bits so we may be ok. However I need to
>>>> re-write my test case which assumed it was 24bits in 3 bytes (S24_3LE).
>>>>
>>>> I'll follow up later,
>>>
>>> Okay, the S24_3LE isn't supported by RT5640 codec in my case. I briefly
>>> looked through the TRM doc and got impression that AHUB could re-pack
>>> data stream into something that codec supports, but maybe it's a wrong
>>> impression.
>>> _________________________________
>>
>> I did a quick test with the following:
>>
>>  sox -n -b 16 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
>>  sox -n -b 24 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
>>  sox -n -b 32 -c 2 -r 44100 /tmp/tmp.wav  synth sine 500 vol 0.5
>>
>> The 16 and 32 work fine, the 24 is showing a playback output freq
>> of 440Hz instead of 500Hz... this suggests the clock is off, or there
>> is something else weird going on...
> 
> I was looking at using sox to create such as file, but the above command
> generates a S24_3LE file and not S24_LE file. The codec on Jetson-TK1
> supports S24_LE but does not support S24_3LE and so I cannot test this.
> Anyway, we really need to test S24_LE and not S24_3LE because this is
> the problem that Dmitry is having.
> 
> Ben is S24_3LE what you really need to support?
> 
> Dmitry, does the following fix your problem?
> 
> diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c
> index dbed3c5408e7..92845c4b63f4 100644
> --- a/sound/soc/tegra/tegra30_i2s.c
> +++ b/sound/soc/tegra/tegra30_i2s.c
> @@ -140,7 +140,7 @@ static int tegra30_i2s_hw_params(struct
> snd_pcm_substream *substream,
>                 audio_bits = TEGRA30_AUDIOCIF_BITS_16;
>                 sample_size = 16;
>                 break;
> -       case SNDRV_PCM_FORMAT_S24_LE:
> +       case SNDRV_PCM_FORMAT_S24_3LE:
>                 val = TEGRA30_I2S_CTRL_BIT_SIZE_24;
>                 audio_bits = TEGRA30_AUDIOCIF_BITS_24;
>                 sample_size = 24;
> @@ -318,7 +318,7 @@ static const struct snd_soc_dai_driver
> tegra30_i2s_dai_template = {
>                 .channels_max = 2,
>                 .rates = SNDRV_PCM_RATE_8000_96000,
>                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
> -                          SNDRV_PCM_FMTBIT_S24_LE |
> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>                            SNDRV_PCM_FMTBIT_S16_LE,
>         },
>         .capture = {
> @@ -327,7 +327,7 @@ static const struct snd_soc_dai_driver
> tegra30_i2s_dai_template = {
>                 .channels_max = 2,
>                 .rates = SNDRV_PCM_RATE_8000_96000,
>                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
> -                          SNDRV_PCM_FMTBIT_S24_LE |
> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>                            SNDRV_PCM_FMTBIT_S16_LE,
>         },
>         .ops = &tegra30_i2s_dai_ops,
> 
> Jon
> 

It should solve the problem in my particular case, but I'm not sure that
the solution is correct.

The v5.5 kernel is released now with the broken audio and apparently
getting 24bit to work won't be trivial (if possible at all). Ben, could
you please send a patch to fix v5.5 by removing the S24 support
advertisement from the driver?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1209 bytes --]

On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
> 24.01.2020 19:50, Jon Hunter пишет:

> >                 .rates = SNDRV_PCM_RATE_8000_96000,
> >                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
> > -                          SNDRV_PCM_FMTBIT_S24_LE |
> > +                          SNDRV_PCM_FMTBIT_S24_3LE |

> It should solve the problem in my particular case, but I'm not sure that
> the solution is correct.

If the format implemented by the driver is S24_3LE the driver should
advertise S24_3LE.

> The v5.5 kernel is released now with the broken audio and apparently
> getting 24bit to work won't be trivial (if possible at all). Ben, could
> you please send a patch to fix v5.5 by removing the S24 support
> advertisement from the driver?

Why is that the best fix rather than just advertising the format
implemented by the driver?

I really don't understand why this is all taking so long, this thread
just seems to be going round in interminable circles long after it
looked like the issue was understood.  I have to admit I've not read
every single message in the thread but it's difficult to see why it
doesn't seem to be making any progress.

[-- 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

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

28.01.2020 15:13, Mark Brown пишет:
> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>> 24.01.2020 19:50, Jon Hunter пишет:
> 
>>>                 .rates = SNDRV_PCM_RATE_8000_96000,
>>>                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
> 
>> It should solve the problem in my particular case, but I'm not sure that
>> the solution is correct.
> 
> If the format implemented by the driver is S24_3LE the driver should
> advertise S24_3LE.

It should be S24_LE, but seems we still don't know for sure.

>> The v5.5 kernel is released now with the broken audio and apparently
>> getting 24bit to work won't be trivial (if possible at all). Ben, could
>> you please send a patch to fix v5.5 by removing the S24 support
>> advertisement from the driver?
> 
> Why is that the best fix rather than just advertising the format
> implemented by the driver?

The currently supported format that is known to work well is S16_LE.

I'm suggesting to drop the S24_LE and S32_LE that were added by the
applied patches simply because this series wasn't tested properly before
it was sent out and turned out that it doesn't work well.

> I really don't understand why this is all taking so long, this thread
> just seems to be going round in interminable circles long after it
> looked like the issue was understood.  I have to admit I've not read
> every single message in the thread but it's difficult to see why it
> doesn't seem to be making any progress.

Ben was trying to make a fix for the introduced problem, but it's not
easy as we see now.

Perhaps the best solution should be to revert all of the three applied
patches and try again later on, once all current problems will be resolved.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Jon Hunter]

On 28/01/2020 17:42, Dmitry Osipenko wrote:
> 28.01.2020 15:13, Mark Brown пишет:
>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>> 24.01.2020 19:50, Jon Hunter пишет:
>>
>>>>                 .rates = SNDRV_PCM_RATE_8000_96000,
>>>>                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>
>>> It should solve the problem in my particular case, but I'm not sure that
>>> the solution is correct.
>>
>> If the format implemented by the driver is S24_3LE the driver should
>> advertise S24_3LE.
> 
> It should be S24_LE, but seems we still don't know for sure.

Why?

>>> The v5.5 kernel is released now with the broken audio and apparently
>>> getting 24bit to work won't be trivial (if possible at all). Ben, could
>>> you please send a patch to fix v5.5 by removing the S24 support
>>> advertisement from the driver?
>>
>> Why is that the best fix rather than just advertising the format
>> implemented by the driver?
> 
> The currently supported format that is known to work well is S16_LE.
> 
> I'm suggesting to drop the S24_LE and S32_LE that were added by the
> applied patches simply because this series wasn't tested properly before
> it was sent out and turned out that it doesn't work well.

S32_LE should be fine, however, I do have some concerns about S24_LE.

Jon

-- 
nvpublic
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [Linux-kernel] [PATCH v5 2/7] ASoC: tegra: Allow 24bit and 32bit samples

[Dmitry Osipenko]

28.01.2020 21:19, Jon Hunter пишет:
> 
> On 28/01/2020 17:42, Dmitry Osipenko wrote:
>> 28.01.2020 15:13, Mark Brown пишет:
>>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>>> 24.01.2020 19:50, Jon Hunter пишет:
>>>
>>>>>                 .rates = SNDRV_PCM_RATE_8000_96000,
>>>>>                 .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>>
>>>> It should solve the problem in my particular case, but I'm not sure that
>>>> the solution is correct.
>>>
>>> If the format implemented by the driver is S24_3LE the driver should
>>> advertise S24_3LE.
>>
>> It should be S24_LE, but seems we still don't know for sure.
> 
> Why?
/I think/ sound should be much more distorted if it was S24_3LE, but
maybe I'm wrong.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Ben Dooks]

On 29/01/2020 00:17, Dmitry Osipenko wrote:
> 28.01.2020 21:19, Jon Hunter пишет:
>>
>> On 28/01/2020 17:42, Dmitry Osipenko wrote:
>>> 28.01.2020 15:13, Mark Brown пишет:
>>>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>>>> 24.01.2020 19:50, Jon Hunter пишет:
>>>>
>>>>>>                  .rates = SNDRV_PCM_RATE_8000_96000,
>>>>>>                  .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>>>
>>>>> It should solve the problem in my particular case, but I'm not sure that
>>>>> the solution is correct.
>>>>
>>>> If the format implemented by the driver is S24_3LE the driver should
>>>> advertise S24_3LE.
>>>
>>> It should be S24_LE, but seems we still don't know for sure.
>>
>> Why?
> /I think/ sound should be much more distorted if it was S24_3LE, but
> maybe I'm wrong.

S24_3LE is IICC the wrong thing and we can't support it on the tegra3


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Clemens Ladisch]

Ben Dooks wrote:
> On 29/01/2020 00:17, Dmitry Osipenko wrote:
>> 28.01.2020 21:19, Jon Hunter пишет:
>>> On 28/01/2020 17:42, Dmitry Osipenko wrote:
>>>> 28.01.2020 15:13, Mark Brown пишет:
>>>>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>>>>> 24.01.2020 19:50, Jon Hunter пишет:
>>>>>
>>>>>>>                  .rates = SNDRV_PCM_RATE_8000_96000,
>>>>>>>                  .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>>>>
>>>>>> It should solve the problem in my particular case, but I'm not sure that
>>>>>> the solution is correct.
>>>>>
>>>>> If the format implemented by the driver is S24_3LE the driver should
>>>>> advertise S24_3LE.
>>>>
>>>> It should be S24_LE, but seems we still don't know for sure.
>>>
>>> Why?
>> /I think/ sound should be much more distorted if it was S24_3LE, but
>> maybe I'm wrong.
>
> S24_3LE is IICC the wrong thing and we can't support it on the tegra3

There are three ways of arranging 24-bit samples in memory:

S24_3LE: 24-bit samples in 24-bit words without padding
S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
S32_LE:  24-bit samples in 32-bit words, aligned to the MSB

It is very unlikely that your hardware implements S24_LE.

If a S32_LE driver wants to describe how many bits are actually used, it
should install a msbits constraint (snd_pcm_hw_constraint_msbits()).


Regards,
Clemens
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Ben Dooks]

On 30/01/2020 09:31, Clemens Ladisch wrote:
> Ben Dooks wrote:
>> On 29/01/2020 00:17, Dmitry Osipenko wrote:
>>> 28.01.2020 21:19, Jon Hunter пишет:
>>>> On 28/01/2020 17:42, Dmitry Osipenko wrote:
>>>>> 28.01.2020 15:13, Mark Brown пишет:
>>>>>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>>>>>> 24.01.2020 19:50, Jon Hunter пишет:
>>>>>>
>>>>>>>>                   .rates = SNDRV_PCM_RATE_8000_96000,
>>>>>>>>                   .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>>>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>>>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>>>>>
>>>>>>> It should solve the problem in my particular case, but I'm not sure that
>>>>>>> the solution is correct.
>>>>>>
>>>>>> If the format implemented by the driver is S24_3LE the driver should
>>>>>> advertise S24_3LE.
>>>>>
>>>>> It should be S24_LE, but seems we still don't know for sure.
>>>>
>>>> Why?
>>> /I think/ sound should be much more distorted if it was S24_3LE, but
>>> maybe I'm wrong.
>>
>> S24_3LE is IICC the wrong thing and we can't support it on the tegra3
> 
> There are three ways of arranging 24-bit samples in memory:
> 
> S24_3LE: 24-bit samples in 24-bit words without padding
> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
> 
> It is very unlikely that your hardware implements S24_LE.

Which is wrong, since this is exactly what the hardware implements.

The DMA fetches 32 bit words, and then the front end dispatches only
24 bits of these to the I2S/TDM

> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Ben Dooks]

On 30/01/2020 09:31, Clemens Ladisch wrote:
> Ben Dooks wrote:
>> On 29/01/2020 00:17, Dmitry Osipenko wrote:
>>> 28.01.2020 21:19, Jon Hunter пишет:
>>>> On 28/01/2020 17:42, Dmitry Osipenko wrote:
>>>>> 28.01.2020 15:13, Mark Brown пишет:
>>>>>> On Mon, Jan 27, 2020 at 10:20:25PM +0300, Dmitry Osipenko wrote:
>>>>>>> 24.01.2020 19:50, Jon Hunter пишет:
>>>>>>
>>>>>>>>                   .rates = SNDRV_PCM_RATE_8000_96000,
>>>>>>>>                   .formats = SNDRV_PCM_FMTBIT_S32_LE |
>>>>>>>> -                          SNDRV_PCM_FMTBIT_S24_LE |
>>>>>>>> +                          SNDRV_PCM_FMTBIT_S24_3LE |
>>>>>>
>>>>>>> It should solve the problem in my particular case, but I'm not sure that
>>>>>>> the solution is correct.
>>>>>>
>>>>>> If the format implemented by the driver is S24_3LE the driver should
>>>>>> advertise S24_3LE.
>>>>>
>>>>> It should be S24_LE, but seems we still don't know for sure.
>>>>
>>>> Why?
>>> /I think/ sound should be much more distorted if it was S24_3LE, but
>>> maybe I'm wrong.
>>
>> S24_3LE is IICC the wrong thing and we can't support it on the tegra3
> 
> There are three ways of arranging 24-bit samples in memory:
> 
> S24_3LE: 24-bit samples in 24-bit words without padding
> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
> 
> It is very unlikely that your hardware implements S24_LE.

Which is wrong, since this is exactly what the hardware implements.

The DMA fetches 32 bit words, and then the front end dispatches only
24 bits of these to the I2S/TDM

> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [alsa-devel] (no subject)

[Clemens Ladisch]

Ben Dooks wrote:
> On 30/01/2020 09:31, Clemens Ladisch wrote:
>> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
>> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
>>
>> It is very unlikely that your hardware implements S24_LE.
>
> Which is wrong, since this is exactly what the hardware implements.
>
> The DMA fetches 32 bit words, and then the front end dispatches only
> 24 bits of these to the I2S/TDM

Which 24 bits?  Is the padding in the first byte (S32_LE) or in the
last byte (S24_LE)?


Regards,
Clemens
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Clemens Ladisch]

Ben Dooks wrote:
> On 30/01/2020 09:31, Clemens Ladisch wrote:
>> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
>> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
>>
>> It is very unlikely that your hardware implements S24_LE.
>
> Which is wrong, since this is exactly what the hardware implements.
>
> The DMA fetches 32 bit words, and then the front end dispatches only
> 24 bits of these to the I2S/TDM

Which 24 bits?  Is the padding in the first byte (S32_LE) or in the
last byte (S24_LE)?


Regards,
Clemens

Re: [alsa-devel] (no subject)

[Ben Dooks]

On 2020-01-30 14:58, Clemens Ladisch wrote:
> Ben Dooks wrote:
>> On 30/01/2020 09:31, Clemens Ladisch wrote:
>>> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
>>> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
>>> 
>>> It is very unlikely that your hardware implements S24_LE.
>> 
>> Which is wrong, since this is exactly what the hardware implements.
>> 
>> The DMA fetches 32 bit words, and then the front end dispatches only
>> 24 bits of these to the I2S/TDM
> 
> Which 24 bits?  Is the padding in the first byte (S32_LE) or in the
> last byte (S24_LE)?

I think the low 24 bits are sent from the 32 bit word.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] (no subject)

[Ben Dooks]

On 2020-01-30 14:58, Clemens Ladisch wrote:
> Ben Dooks wrote:
>> On 30/01/2020 09:31, Clemens Ladisch wrote:
>>> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
>>> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
>>> 
>>> It is very unlikely that your hardware implements S24_LE.
>> 
>> Which is wrong, since this is exactly what the hardware implements.
>> 
>> The DMA fetches 32 bit words, and then the front end dispatches only
>> 24 bits of these to the I2S/TDM
> 
> Which 24 bits?  Is the padding in the first byte (S32_LE) or in the
> last byte (S24_LE)?

I think the low 24 bits are sent from the 32 bit word.

Re: [alsa-devel] (no subject)

[Clemens Ladisch]

Ben Dooks wrote:
> On 2020-01-30 14:58, Clemens Ladisch wrote:
>> Ben Dooks wrote:
>>> On 30/01/2020 09:31, Clemens Ladisch wrote:
>>>> S24_LE:  24-bit samples in 32-bit words, aligned to the LSB
>>>> S32_LE:  24-bit samples in 32-bit words, aligned to the MSB
>>>>
>>>> It is very unlikely that your hardware implements S24_LE.
>>>
>>> Which is wrong, since this is exactly what the hardware implements.
>>>
>>> The DMA fetches 32 bit words, and then the front end dispatches only
>>> 24 bits of these to the I2S/TDM
>>
>> Which 24 bits?  Is the padding in the first byte (S32_LE) or in the
>> last byte (S24_LE)?
>
> I think the low 24 bits are sent from the 32 bit word.

This would indeed by S24_LE.  If you change the driver to say that it
supports _only_ S24_LE, is the data then played correctly?


Regards,
Clemens
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

(no subject)

[Bryan Wu]

Re: [alsa-devel] (no subject)

[Mark Brown]

On Tue, Nov 18, 2008 at 04:18:14PM +0800, Bryan Wu wrote:

...nothing.  :)

All patches look OK, a couple of comments to follow.