[alsa-devel] [PATCH 0/2] ASoC: mediatek: mt8183-da7219: add separate speaker switch

[alsa-devel] [PATCH 0/2] ASoC: mediatek: mt8183-da7219: add separate speaker switch

[Tzung-Bi Shih]

Per previous discussion[1], the patch moves control of SD_MODE from
PCM trigger to DAPM (i.e. set SD_MODE earilier if playback).

Possible drawback: may generate pop noise when BCLK=on but LRCLK=off.
As the datasheet[2] mentioned:
> Do not remove LRCLK while BCLK is present.
> Removing LRCLK while BCLK is present can cause unexpected output behavior
> including a large DC output voltage.

That is also the original purpose of sdmode-delay: to make sure SD_MODE
is the last one to be set.

[1]: https://patchwork.kernel.org/patch/11363953/
[2]: https://datasheets.maximintegrated.com/en/ds/MAX98357A-MAX98357B.pdf

Tzung-Bi Shih (2):
  ASoC: max98357a: move control of SD_MODE to DAPM
  ASoC: mediatek: mt8183-da7219: add speaker switch

 sound/soc/codecs/max98357a.c                  | 36 +++++++++----------
 .../mediatek/mt8183/mt8183-da7219-max98357.c  | 19 ++++++++++
 2 files changed, 36 insertions(+), 19 deletions(-)

-- 
2.25.0.225.g125e21ebc7-goog

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

[alsa-devel] [PATCH 1/2] ASoC: max98357a: move control of SD_MODE to DAPM

[Tzung-Bi Shih]

Some machine may share the same I2S lines for multiple codecs. For
example, mediatek/mt8183/mt8183-da7219-max98357 shares the same lines
between max98357a and da7219.  When writing audio data through the I2S
lines, all codecs on the lines would try to generate sound if they
accepts DO line.  As a result, multiple codecs generate sound at a
time.

Moves control of SD_MODE to DAPM so that machine drivers have chances
to manipulate DAPM widget to turn on/off max98357a.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 sound/soc/codecs/max98357a.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c
index 16313b973eaa..74f20114297c 100644
--- a/sound/soc/codecs/max98357a.c
+++ b/sound/soc/codecs/max98357a.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
@@ -24,26 +25,24 @@ struct max98357a_priv {
 	unsigned int sdmode_delay;
 };
 
-static int max98357a_daiops_trigger(struct snd_pcm_substream *substream,
-		int cmd, struct snd_soc_dai *dai)
+static int max98357a_sdmode_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
 {
-	struct max98357a_priv *max98357a = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *component =
+		snd_soc_dapm_to_component(w->dapm);
+	struct max98357a_priv *max98357a =
+		snd_soc_component_get_drvdata(component);
 
 	if (!max98357a->sdmode)
 		return 0;
 
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		mdelay(max98357a->sdmode_delay);
+	if (event & SND_SOC_DAPM_POST_PMU) {
+		msleep(max98357a->sdmode_delay);
 		gpiod_set_value(max98357a->sdmode, 1);
-		break;
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		dev_dbg(component->dev, "set sdmode to 1");
+	} else if (event & SND_SOC_DAPM_PRE_PMD) {
 		gpiod_set_value(max98357a->sdmode, 0);
-		break;
+		dev_dbg(component->dev, "set sdmode to 0");
 	}
 
 	return 0;
@@ -51,10 +50,14 @@ static int max98357a_daiops_trigger(struct snd_pcm_substream *substream,
 
 static const struct snd_soc_dapm_widget max98357a_dapm_widgets[] = {
 	SND_SOC_DAPM_OUTPUT("Speaker"),
+	SND_SOC_DAPM_OUT_DRV_E("SD_MODE", SND_SOC_NOPM, 0, 0, NULL, 0,
+			max98357a_sdmode_event,
+			SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
 };
 
 static const struct snd_soc_dapm_route max98357a_dapm_routes[] = {
-	{"Speaker", NULL, "HiFi Playback"},
+	{"SD_MODE", NULL, "HiFi Playback"},
+	{"Speaker", NULL, "SD_MODE"},
 };
 
 static const struct snd_soc_component_driver max98357a_component_driver = {
@@ -68,10 +71,6 @@ static const struct snd_soc_component_driver max98357a_component_driver = {
 	.non_legacy_dai_naming	= 1,
 };
 
-static const struct snd_soc_dai_ops max98357a_dai_ops = {
-	.trigger	= max98357a_daiops_trigger,
-};
-
 static struct snd_soc_dai_driver max98357a_dai_driver = {
 	.name = "HiFi",
 	.playback = {
@@ -91,7 +90,6 @@ static struct snd_soc_dai_driver max98357a_dai_driver = {
 		.channels_min	= 1,
 		.channels_max	= 2,
 	},
-	.ops    = &max98357a_dai_ops,
 };
 
 static int max98357a_platform_probe(struct platform_device *pdev)
-- 
2.25.0.225.g125e21ebc7-goog

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

[alsa-devel] [PATCH 2/2] ASoC: mediatek: mt8183-da7219: add speaker switch

[Tzung-Bi Shih]

Da7219 and max98357a share the same I2S lines.  When writing audio data
to the I2S, both codecs generate sound.

Da7219 already has a separate control "Headphone Switch".  Adds a new
control "Speakers Switch" for turning on/off max98357a.  Userspace
program can decide to turn on/off which codecs by different use cases.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
 .../mediatek/mt8183/mt8183-da7219-max98357.c  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
index c65493721e90..1626541cc0d6 100644
--- a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
@@ -372,9 +372,28 @@ static struct snd_soc_codec_conf mt6358_codec_conf[] = {
 	},
 };
 
+static const struct snd_kcontrol_new mt8183_da7219_max98357_snd_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Speakers"),
+};
+
+static const
+struct snd_soc_dapm_widget mt8183_da7219_max98357_dapm_widgets[] = {
+	SND_SOC_DAPM_SPK("Speakers", NULL),
+};
+
+static const struct snd_soc_dapm_route mt8183_da7219_max98357_dapm_routes[] = {
+	{"Speakers", NULL, "Speaker"},
+};
+
 static struct snd_soc_card mt8183_da7219_max98357_card = {
 	.name = "mt8183_da7219_max98357",
 	.owner = THIS_MODULE,
+	.controls = mt8183_da7219_max98357_snd_controls,
+	.num_controls = ARRAY_SIZE(mt8183_da7219_max98357_snd_controls),
+	.dapm_widgets = mt8183_da7219_max98357_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(mt8183_da7219_max98357_dapm_widgets),
+	.dapm_routes = mt8183_da7219_max98357_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(mt8183_da7219_max98357_dapm_routes),
 	.dai_link = mt8183_da7219_max98357_dai_links,
 	.num_links = ARRAY_SIZE(mt8183_da7219_max98357_dai_links),
 	.aux_dev = &mt8183_da7219_max98357_headset_dev,
-- 
2.25.0.225.g125e21ebc7-goog

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

[alsa-devel] Applied "ASoC: mediatek: mt8183-da7219: add speaker switch" to the asoc tree

[Mark Brown]

The patch

   ASoC: mediatek: mt8183-da7219: add speaker switch

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 514de1c935d1670e0f162210f3794cf0be67c8a7 Mon Sep 17 00:00:00 2001
From: Tzung-Bi Shih <tzungbi@google.com>
Date: Wed, 12 Feb 2020 13:55:17 +0800
Subject: [PATCH] ASoC: mediatek: mt8183-da7219: add speaker switch

Da7219 and max98357a share the same I2S lines.  When writing audio data
to the I2S, both codecs generate sound.

Da7219 already has a separate control "Headphone Switch".  Adds a new
control "Speakers Switch" for turning on/off max98357a.  Userspace
program can decide to turn on/off which codecs by different use cases.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
Link: https://lore.kernel.org/r/20200212124608.2.I5fa3fdca69dbb5d3dd5031c939b9b24095065a94@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../mediatek/mt8183/mt8183-da7219-max98357.c  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
index c65493721e90..1626541cc0d6 100644
--- a/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
+++ b/sound/soc/mediatek/mt8183/mt8183-da7219-max98357.c
@@ -372,9 +372,28 @@ static struct snd_soc_codec_conf mt6358_codec_conf[] = {
 	},
 };
 
+static const struct snd_kcontrol_new mt8183_da7219_max98357_snd_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Speakers"),
+};
+
+static const
+struct snd_soc_dapm_widget mt8183_da7219_max98357_dapm_widgets[] = {
+	SND_SOC_DAPM_SPK("Speakers", NULL),
+};
+
+static const struct snd_soc_dapm_route mt8183_da7219_max98357_dapm_routes[] = {
+	{"Speakers", NULL, "Speaker"},
+};
+
 static struct snd_soc_card mt8183_da7219_max98357_card = {
 	.name = "mt8183_da7219_max98357",
 	.owner = THIS_MODULE,
+	.controls = mt8183_da7219_max98357_snd_controls,
+	.num_controls = ARRAY_SIZE(mt8183_da7219_max98357_snd_controls),
+	.dapm_widgets = mt8183_da7219_max98357_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(mt8183_da7219_max98357_dapm_widgets),
+	.dapm_routes = mt8183_da7219_max98357_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(mt8183_da7219_max98357_dapm_routes),
 	.dai_link = mt8183_da7219_max98357_dai_links,
 	.num_links = ARRAY_SIZE(mt8183_da7219_max98357_dai_links),
 	.aux_dev = &mt8183_da7219_max98357_headset_dev,
-- 
2.20.1

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

[alsa-devel] Applied "ASoC: max98357a: move control of SD_MODE to DAPM" to the asoc tree

[Mark Brown]

The patch

   ASoC: max98357a: move control of SD_MODE to DAPM

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 128f825aeab79ebff9679a84f49105eda85ecf2c Mon Sep 17 00:00:00 2001
From: Tzung-Bi Shih <tzungbi@google.com>
Date: Wed, 12 Feb 2020 13:55:16 +0800
Subject: [PATCH] ASoC: max98357a: move control of SD_MODE to DAPM

Some machine may share the same I2S lines for multiple codecs. For
example, mediatek/mt8183/mt8183-da7219-max98357 shares the same lines
between max98357a and da7219.  When writing audio data through the I2S
lines, all codecs on the lines would try to generate sound if they
accepts DO line.  As a result, multiple codecs generate sound at a
time.

Moves control of SD_MODE to DAPM so that machine drivers have chances
to manipulate DAPM widget to turn on/off max98357a.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
Link: https://lore.kernel.org/r/20200212124608.1.I73b26b5e319de173d05823e79f5861bf1826261c@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/max98357a.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c
index 16313b973eaa..74f20114297c 100644
--- a/sound/soc/codecs/max98357a.c
+++ b/sound/soc/codecs/max98357a.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
@@ -24,26 +25,24 @@ struct max98357a_priv {
 	unsigned int sdmode_delay;
 };
 
-static int max98357a_daiops_trigger(struct snd_pcm_substream *substream,
-		int cmd, struct snd_soc_dai *dai)
+static int max98357a_sdmode_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
 {
-	struct max98357a_priv *max98357a = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *component =
+		snd_soc_dapm_to_component(w->dapm);
+	struct max98357a_priv *max98357a =
+		snd_soc_component_get_drvdata(component);
 
 	if (!max98357a->sdmode)
 		return 0;
 
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		mdelay(max98357a->sdmode_delay);
+	if (event & SND_SOC_DAPM_POST_PMU) {
+		msleep(max98357a->sdmode_delay);
 		gpiod_set_value(max98357a->sdmode, 1);
-		break;
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		dev_dbg(component->dev, "set sdmode to 1");
+	} else if (event & SND_SOC_DAPM_PRE_PMD) {
 		gpiod_set_value(max98357a->sdmode, 0);
-		break;
+		dev_dbg(component->dev, "set sdmode to 0");
 	}
 
 	return 0;
@@ -51,10 +50,14 @@ static int max98357a_daiops_trigger(struct snd_pcm_substream *substream,
 
 static const struct snd_soc_dapm_widget max98357a_dapm_widgets[] = {
 	SND_SOC_DAPM_OUTPUT("Speaker"),
+	SND_SOC_DAPM_OUT_DRV_E("SD_MODE", SND_SOC_NOPM, 0, 0, NULL, 0,
+			max98357a_sdmode_event,
+			SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
 };
 
 static const struct snd_soc_dapm_route max98357a_dapm_routes[] = {
-	{"Speaker", NULL, "HiFi Playback"},
+	{"SD_MODE", NULL, "HiFi Playback"},
+	{"Speaker", NULL, "SD_MODE"},
 };
 
 static const struct snd_soc_component_driver max98357a_component_driver = {
@@ -68,10 +71,6 @@ static const struct snd_soc_component_driver max98357a_component_driver = {
 	.non_legacy_dai_naming	= 1,
 };
 
-static const struct snd_soc_dai_ops max98357a_dai_ops = {
-	.trigger	= max98357a_daiops_trigger,
-};
-
 static struct snd_soc_dai_driver max98357a_dai_driver = {
 	.name = "HiFi",
 	.playback = {
@@ -91,7 +90,6 @@ static struct snd_soc_dai_driver max98357a_dai_driver = {
 		.channels_min	= 1,
 		.channels_max	= 2,
 	},
-	.ops    = &max98357a_dai_ops,
 };
 
 static int max98357a_platform_probe(struct platform_device *pdev)
-- 
2.20.1

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