From a97962111e6710adb127bff28092236687b742c3 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 4 Sep 2018 17:58:34 +0200
Subject: [PATCH] staging: bcm2835-audio: Fix mute controls, volume handling cleanup
Git-commit: a97962111e6710adb127bff28092236687b742c3
Patch-mainline: v4.20-rc1
References: FATE#324827

In the current code, the mute control is dealt in a special manner,
modifying the current volume and saving the old volume, etc.  This is
inconsistent (e.g. change the volume while muted, then unmute), and
way too complex.

Also, the whole volume handling code has conversion between ALSA
volume and raw volume values, which can lead to another
inconsistency and complexity.

This patch simplifies these points:
- The ALSA volume value is saved in chip->volume
- volume->mute saves the mute state
- The mute state is evaluated only when the actual volume is passed to
  the hardware, bcm2835_audio_set_ctls()

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c   |   84 ++++--------
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c   |    6 
 drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c |   32 +---
 drivers/staging/vc04_services/bcm2835-audio/bcm2835.h       |    5 
 4 files changed, 45 insertions(+), 82 deletions(-)

--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
@@ -12,6 +12,21 @@
 #define CTRL_VOL_MAX 400
 #define CTRL_VOL_MIN -10239 /* originally -10240 */
 
+static int bcm2835_audio_set_chip_ctls(struct bcm2835_chip *chip)
+{
+	int i, err = 0;
+
+	/* change ctls for all substreams */
+	for (i = 0; i < MAX_SUBSTREAMS; i++) {
+		if (chip->alsa_stream[i]) {
+			err = bcm2835_audio_set_ctls(chip->alsa_stream[i]);
+			if (err < 0)
+				break;
+		}
+	}
+	return err;
+}
+
 static int snd_bcm2835_ctl_info(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_info *uinfo)
 {
@@ -34,29 +49,6 @@ static int snd_bcm2835_ctl_info(struct s
 	return 0;
 }
 
-/* toggles mute on or off depending on the value of nmute, and returns
- * 1 if the mute value was changed, otherwise 0
- */
-static int toggle_mute(struct bcm2835_chip *chip, int nmute)
-{
-	/* if settings are ok, just return 0 */
-	if (chip->mute == nmute)
-		return 0;
-
-	/* if the sound is muted then we need to unmute */
-	if (chip->mute == CTRL_VOL_MUTE) {
-		chip->volume = chip->old_volume; /* copy the old volume back */
-		audio_info("Unmuting, old_volume = %d, volume = %d ...\n", chip->old_volume, chip->volume);
-	} else /* otherwise we mute */ {
-		chip->old_volume = chip->volume;
-		chip->volume = 26214; /* set volume to minimum level AKA mute */
-		audio_info("Muting, old_volume = %d, volume = %d ...\n", chip->old_volume, chip->volume);
-	}
-
-	chip->mute = nmute;
-	return 1;
-}
-
 static int snd_bcm2835_ctl_get(struct snd_kcontrol *kcontrol,
 			       struct snd_ctl_elem_value *ucontrol)
 {
@@ -65,7 +57,7 @@ static int snd_bcm2835_ctl_get(struct sn
 	mutex_lock(&chip->audio_mutex);
 
 	if (kcontrol->private_value == PCM_PLAYBACK_VOLUME)
-		ucontrol->value.integer.value[0] = chip2alsa(chip->volume);
+		ucontrol->value.integer.value[0] = chip->volume;
 	else if (kcontrol->private_value == PCM_PLAYBACK_MUTE)
 		ucontrol->value.integer.value[0] = chip->mute;
 	else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE)
@@ -79,38 +71,26 @@ static int snd_bcm2835_ctl_put(struct sn
 				struct snd_ctl_elem_value *ucontrol)
 {
 	struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
+	int val, *valp;
 	int changed = 0;
 
-	mutex_lock(&chip->audio_mutex);
-
-	if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) {
-		audio_info("Volume change attempted.. volume = %d new_volume = %d\n", chip->volume, (int)ucontrol->value.integer.value[0]);
-		if (chip->mute == CTRL_VOL_MUTE) {
-			/* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */
-			changed = 1; /* should return 0 to signify no change but the mixer takes this as the opposite sign (no idea why) */
-			goto unlock;
-		}
-		if (changed || (ucontrol->value.integer.value[0] != chip2alsa(chip->volume))) {
-			chip->volume = alsa2chip(ucontrol->value.integer.value[0]);
-			changed = 1;
-		}
-
-	} else if (kcontrol->private_value == PCM_PLAYBACK_MUTE) {
-		/* Now implemented */
-		audio_info(" Mute attempted\n");
-		changed = toggle_mute(chip, ucontrol->value.integer.value[0]);
+	if (kcontrol->private_value == PCM_PLAYBACK_VOLUME)
+		valp = &chip->volume;
+	else if (kcontrol->private_value == PCM_PLAYBACK_MUTE)
+		valp = &chip->mute;
+	else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE)
+		valp = &chip->dest;
+	else
+		return -EINVAL;
 
-	} else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE) {
-		if (ucontrol->value.integer.value[0] != chip->dest) {
-			chip->dest = ucontrol->value.integer.value[0];
-			changed = 1;
-		}
+	val = ucontrol->value.integer.value[0];
+	mutex_lock(&chip->audio_mutex);
+	if (val != *valp) {
+		*valp = val;
+		changed = 1;
+		if (bcm2835_audio_set_chip_ctls(chip))
+			dev_err(chip->card->dev, "Failed to set ALSA controls..\n");
 	}
-
-	if (changed && bcm2835_audio_set_ctls(chip))
-		dev_err(chip->card->dev, "Failed to set ALSA controls..\n");
-
-unlock:
 	mutex_unlock(&chip->audio_mutex);
 	return changed;
 }
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
@@ -280,7 +280,7 @@ static int snd_bcm2835_pcm_prepare(struc
 	bcm2835_audio_setup(alsa_stream);
 
 	/* in preparation of the stream, set the controls (volume level) of the stream */
-	bcm2835_audio_set_ctls(alsa_stream->chip);
+	bcm2835_audio_set_ctls(alsa_stream);
 
 	memset(&alsa_stream->pcm_indirect, 0, sizeof(alsa_stream->pcm_indirect));
 
@@ -441,7 +441,7 @@ int snd_bcm2835_new_pcm(struct bcm2835_c
 	strcpy(pcm->name, "bcm2835 ALSA");
 	chip->pcm = pcm;
 	chip->dest = AUDIO_DEST_AUTO;
-	chip->volume = alsa2chip(0);
+	chip->volume = 0;
 	chip->mute = CTRL_VOL_UNMUTE; /*disable mute on startup */
 	/* set operators */
 	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK,
@@ -498,7 +498,7 @@ int snd_bcm2835_new_simple_pcm(struct bc
 	strcpy(pcm->name, name);
 	chip->pcm = pcm;
 	chip->dest = route;
-	chip->volume = alsa2chip(0);
+	chip->volume = 0;
 	chip->mute = CTRL_VOL_UNMUTE;
 
 	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK,
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -460,11 +460,11 @@ free_wq:
 	return ret;
 }
 
-static int bcm2835_audio_set_ctls_chan(struct bcm2835_alsa_stream *alsa_stream,
-				       struct bcm2835_chip *chip)
+int bcm2835_audio_set_ctls(struct bcm2835_alsa_stream *alsa_stream)
 {
 	struct vc_audio_msg m;
 	struct bcm2835_audio_instance *instance = alsa_stream->instance;
+	struct bcm2835_chip *chip = alsa_stream->chip;
 	int status;
 	int ret;
 
@@ -478,7 +478,10 @@ static int bcm2835_audio_set_ctls_chan(s
 
 	m.type = VC_AUDIO_MSG_TYPE_CONTROL;
 	m.u.control.dest = chip->dest;
-	m.u.control.volume = chip->volume;
+	if (!chip->mute)
+		m.u.control.volume = CHIP_MIN_VOLUME;
+	else
+		m.u.control.volume = alsa2chip(chip->volume);
 
 	/* Create the message available completion */
 	init_completion(&instance->msg_avail_comp);
@@ -514,27 +517,6 @@ unlock:
 	return ret;
 }
 
-int bcm2835_audio_set_ctls(struct bcm2835_chip *chip)
-{
-	int i;
-	int ret = 0;
-
-	LOG_DBG(" Setting ALSA dest(%d), volume(%d)\n", chip->dest, chip->volume);
-
-	/* change ctls for all substreams */
-	for (i = 0; i < MAX_SUBSTREAMS; i++) {
-		if (!chip->alsa_stream[i])
-			continue;
-		if (bcm2835_audio_set_ctls_chan(chip->alsa_stream[i], chip) != 0) {
-			LOG_ERR("Couldn't set the controls for stream %d\n", i);
-			ret = -1;
-		} else {
-			LOG_DBG(" Controls set for stream %d\n", i);
-		}
-	}
-	return ret;
-}
-
 int bcm2835_audio_set_params(struct bcm2835_alsa_stream *alsa_stream,
 			     unsigned int channels, unsigned int samplerate,
 			     unsigned int bps)
@@ -548,7 +530,7 @@ int bcm2835_audio_set_params(struct bcm2
 		 channels, samplerate, bps);
 
 	/* resend ctls - alsa_stream may not have been open when first send */
-	ret = bcm2835_audio_set_ctls_chan(alsa_stream, alsa_stream->chip);
+	ret = bcm2835_audio_set_ctls(alsa_stream);
 	if (ret) {
 		LOG_ERR(" Alsa controls not supported\n");
 		return -EINVAL;
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
@@ -74,6 +74,8 @@ enum {
 // convert chip to alsa volume
 #define chip2alsa(vol) -(((vol) * 100) >> 8)
 
+#define CHIP_MIN_VOLUME		26214 /* minimum level aka mute */
+
 /* Some constants for values .. */
 enum snd_bcm2835_route {
 	AUDIO_DEST_AUTO = 0,
@@ -102,7 +104,6 @@ struct bcm2835_chip {
 	struct bcm2835_alsa_stream *alsa_stream[MAX_SUBSTREAMS];
 
 	int volume;
-	int old_volume; /* stores the volume value whist muted */
 	int dest;
 	int mute;
 
@@ -160,7 +161,7 @@ int bcm2835_audio_set_params(struct bcm2
 int bcm2835_audio_setup(struct bcm2835_alsa_stream *alsa_stream);
 int bcm2835_audio_start(struct bcm2835_alsa_stream *alsa_stream);
 int bcm2835_audio_stop(struct bcm2835_alsa_stream *alsa_stream);
-int bcm2835_audio_set_ctls(struct bcm2835_chip *chip);
+int bcm2835_audio_set_ctls(struct bcm2835_alsa_stream *alsa_stream);
 int bcm2835_audio_write(struct bcm2835_alsa_stream *alsa_stream,
 			unsigned int count,
 			void *src);