diff mbox series

ASoC: max98357a: move control of SD_MODE back to DAI ops

Message ID 20200721114232.2812254-1-tzungbi@google.com (mailing list archive)
State New, archived
Headers show
Series ASoC: max98357a: move control of SD_MODE back to DAI ops | expand

Commit Message

Tzung-Bi Shih July 21, 2020, 11:42 a.m. UTC
Partially reverts commit 128f825aeab7 ("ASoC: max98357a: move control
of SD_MODE to DAPM").

In order to have mute control of max98357 from machine drivers, commit
128f825aeab7 ("ASoC: max98357a: move control of SD_MODE to DAPM")
moves the control of SD_MODE from DAI ops to DAPM events.  However, pop
noise has been observed on rk3399-gru-kevin boards due to this commit.

The commit 128f825aeab7 caused sequence of DAI clocks and SD_MODE
changed on rk3399-gru-kevin boards.

With the commit 128f825aeab7:
- SD_MODE will be set to 1 before DAI clocks start.
- SD_MODE will be set to 0 after DAI clocks stop.
As a result, pop noise.

Moves the control of SD_MODE back to DAI ops.  In the meantime, uses an
additional flag in DAPM event to provide chance of mute control for
machine drivers.

Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---

This patch addresses the reported issue in the thread
https://lkml.org/lkml/2020/7/16/427

 sound/soc/codecs/max98357a.c | 50 ++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 10 deletions(-)

Comments

Alper Nebi Yasak July 21, 2020, 1:25 p.m. UTC | #1
On 21/07/2020 14:42, Tzung-Bi Shih wrote:
> This patch addresses the reported issue in the thread
> https://lkml.org/lkml/2020/7/16/427

I can confirm this patch eliminates the pop sounds I mentioned in that
thread. Thanks for working on it.

It looks like toggling "Speaker Switch" during playback doesn't take
effect until playback finishes, but I found that PulseAudio (a version
from a merge request with a custom UCM config) can switch between
Speakers and Headphones immediately, so I don't really think it'll be a
problem.

Tested-By: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Mark Brown July 22, 2020, 12:57 a.m. UTC | #2
On Tue, 21 Jul 2020 19:42:32 +0800, Tzung-Bi Shih wrote:
> Partially reverts commit 128f825aeab7 ("ASoC: max98357a: move control
> of SD_MODE to DAPM").
> 
> In order to have mute control of max98357 from machine drivers, commit
> 128f825aeab7 ("ASoC: max98357a: move control of SD_MODE to DAPM")
> moves the control of SD_MODE from DAI ops to DAPM events.  However, pop
> noise has been observed on rk3399-gru-kevin boards due to this commit.
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: max98357a: move control of SD_MODE back to DAI ops
      commit: 04a646ff5acaa9a0a6634af1c94e0d5c8115e5db

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
diff mbox series

Patch

diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c
index 4f431133d0bb..918812763884 100644
--- a/sound/soc/codecs/max98357a.c
+++ b/sound/soc/codecs/max98357a.c
@@ -23,36 +23,61 @@ 
 struct max98357a_priv {
 	struct gpio_desc *sdmode;
 	unsigned int sdmode_delay;
+	int sdmode_switch;
 };
 
-static int max98357a_sdmode_event(struct snd_soc_dapm_widget *w,
-		struct snd_kcontrol *kcontrol, int event)
+static int max98357a_daiops_trigger(struct snd_pcm_substream *substream,
+		int cmd, struct snd_soc_dai *dai)
 {
-	struct snd_soc_component *component =
-		snd_soc_dapm_to_component(w->dapm);
+	struct snd_soc_component *component = dai->component;
 	struct max98357a_priv *max98357a =
 		snd_soc_component_get_drvdata(component);
 
 	if (!max98357a->sdmode)
 		return 0;
 
-	if (event & SND_SOC_DAPM_POST_PMU) {
-		msleep(max98357a->sdmode_delay);
-		gpiod_set_value(max98357a->sdmode, 1);
-		dev_dbg(component->dev, "set sdmode to 1");
-	} else if (event & SND_SOC_DAPM_PRE_PMD) {
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		mdelay(max98357a->sdmode_delay);
+		if (max98357a->sdmode_switch) {
+			gpiod_set_value(max98357a->sdmode, 1);
+			dev_dbg(component->dev, "set sdmode to 1");
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		gpiod_set_value(max98357a->sdmode, 0);
 		dev_dbg(component->dev, "set sdmode to 0");
+		break;
 	}
 
 	return 0;
 }
 
+static int max98357a_sdmode_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component =
+		snd_soc_dapm_to_component(w->dapm);
+	struct max98357a_priv *max98357a =
+		snd_soc_component_get_drvdata(component);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		max98357a->sdmode_switch = 1;
+	else if (event & SND_SOC_DAPM_POST_PMD)
+		max98357a->sdmode_switch = 0;
+
+	return 0;
+}
+
 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),
+			SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
 };
 
 static const struct snd_soc_dapm_route max98357a_dapm_routes[] = {
@@ -71,6 +96,10 @@  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 = {
@@ -90,6 +119,7 @@  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)