linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Anholt <eric@anholt.net>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	linux-rpi-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 05/29] staging: bcm2835-audio: Fix mute controls, volume handling cleanup
Date: Tue,  4 Sep 2018 17:58:34 +0200	[thread overview]
Message-ID: <20180904155858.8001-6-tiwai@suse.de> (raw)
In-Reply-To: <20180904155858.8001-1-tiwai@suse.de>

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>
---
 .../vc04_services/bcm2835-audio/bcm2835-ctl.c | 84 +++++++------------
 .../vc04_services/bcm2835-audio/bcm2835-pcm.c |  6 +-
 .../bcm2835-audio/bcm2835-vchiq.c             | 32 ++-----
 .../vc04_services/bcm2835-audio/bcm2835.h     |  5 +-
 4 files changed, 45 insertions(+), 82 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
index d2f0f609f737..e17b72f21a9d 100644
--- 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 snd_kcontrol *kcontrol,
 	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 snd_kcontrol *kcontrol,
 	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 snd_kcontrol *kcontrol,
 				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;
 }
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
index 0be185350f33..9a79d2267df4 100644
--- 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(struct snd_pcm_substream *substream)
 	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_chip *chip, u32 numchannels)
 	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 bcm2835_chip *chip,
 	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,
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 942a38942c29..8684dc1d0b41 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -460,11 +460,11 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream)
 	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(struct bcm2835_alsa_stream *alsa_stream,
 
 	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 @@ static int bcm2835_audio_set_ctls_chan(struct bcm2835_alsa_stream *alsa_stream,
 	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 bcm2835_alsa_stream *alsa_stream,
 		 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;
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
index c0e7df4914ed..8ee25a837b08 100644
--- 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 bcm2835_alsa_stream *alsa_stream,
 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);
-- 
2.18.0


  parent reply	other threads:[~2018-09-04 16:00 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 15:58 [PATCH 00/29] staging: bcm2835-audio: Cleanups and fixes Takashi Iwai
2018-09-04 15:58 ` [PATCH 01/29] staging: bcm2835-audio: Clean up mutex locks Takashi Iwai
2018-09-04 15:58 ` [PATCH 02/29] staging: bcm2835-audio: Remove redundant spdif stream ctls Takashi Iwai
2018-09-04 15:58 ` [PATCH 03/29] staging: bcm2835-audio: Clean up include files in bcm2835-ctl.c Takashi Iwai
2018-09-08 13:25   ` Stefan Wahren
2018-09-08 16:21     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 04/29] staging: bcm2835-audio: Remove redundant substream mask checks Takashi Iwai
2018-09-04 15:58 ` Takashi Iwai [this message]
2018-09-04 15:58 ` [PATCH 06/29] staging: bcm2835-audio: Remove redundant function calls Takashi Iwai
2018-09-04 15:58 ` [PATCH 07/29] staging: bcm2835-audio: Remove superfluous open flag Takashi Iwai
2018-09-04 15:58 ` [PATCH 08/29] staging: bcm2835-audio: Drop useless running flag and check Takashi Iwai
2018-09-04 15:58 ` [PATCH 09/29] staging: bcm2835-audio: Fix incorrect draining handling Takashi Iwai
2018-09-04 15:58 ` [PATCH 10/29] staging: bcm2835-audio: Kill unused spinlock Takashi Iwai
2018-09-04 15:58 ` [PATCH 11/29] staging: bcm2835-audio: Use PCM runtime values instead Takashi Iwai
2018-09-04 15:58 ` [PATCH 12/29] staging: bcm2835-audio: Drop unnecessary pcm indirect setup Takashi Iwai
2018-09-04 15:58 ` [PATCH 13/29] staging: bcm2835-audio: Drop useless NULL check Takashi Iwai
2018-09-04 15:58 ` [PATCH 14/29] staging: bcm2835-audio: Propagate parameter setup error Takashi Iwai
2018-09-04 15:58 ` [PATCH 15/29] staging: bcm2835-audio: Drop debug messages in bcm2835-pcm.c Takashi Iwai
2018-09-04 15:58 ` [PATCH 16/29] staging: bcm2835-audio: Drop superfluous mutex lock during prepare Takashi Iwai
2018-09-08 13:40   ` Stefan Wahren
2018-09-08 16:12     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 17/29] staging: bcm2835-audio: Add 10ms period constraint Takashi Iwai
2018-09-19  9:42   ` Stefan Wahren
2018-09-19  9:52     ` Takashi Iwai
2018-09-19 12:41       ` Stefan Wahren
2018-09-19 12:47         ` Mike Brady
2018-09-19 18:39         ` Takashi Iwai
2018-10-09 13:18           ` [PATCH 17/29] staging: bcm2835-audio: Add 10ms period constraint [Resend in plain text...] Mike Brady
2018-10-09 13:44             ` Takashi Iwai
2018-10-09 15:28               ` Mike Brady
2018-10-09 15:32                 ` Takashi Iwai
2018-10-11 12:53                 ` Mike Brady
2018-10-11 14:07                   ` Stefan Wahren
2018-10-13 15:00                   ` Mike Brady
2018-10-13 15:45                     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 18/29] staging: bcm2835-audio: Make single vchi handle Takashi Iwai
2018-09-04 15:58 ` [PATCH 19/29] staging: bcm2835-audio: Code refactoring of vchiq accessor codes Takashi Iwai
2018-09-04 15:58 ` [PATCH 20/29] staging: bcm2835-audio: Operate non-atomic PCM ops Takashi Iwai
2018-09-04 15:58 ` [PATCH 21/29] staging: bcm2835-audio: Use card->private_data Takashi Iwai
2018-09-04 15:58 ` [PATCH 22/29] staging: bcm2835-audio: Use standard error print helpers Takashi Iwai
2018-09-04 15:58 ` [PATCH 23/29] staging: bcm2835-audio: Remove unnecessary header file includes Takashi Iwai
2018-09-04 15:58 ` [PATCH 24/29] staging: bcm2835-audio: Move module parameter description Takashi Iwai
2018-09-04 15:58 ` [PATCH 25/29] staging: bcm2835-audio: Use coherent device buffers Takashi Iwai
2018-09-04 15:58 ` [PATCH 26/29] staging: bcm2835-audio: Set SNDRV_PCM_INFO_SYNC_APPLPTR Takashi Iwai
2018-09-04 15:58 ` [PATCH 27/29] staging: bcm2835-audio: Simplify PCM creation helpers Takashi Iwai
2018-09-04 15:58 ` [PATCH 28/29] staging: bcm2835-audio: Simplify kctl " Takashi Iwai
2018-09-04 15:58 ` [PATCH 29/29] staging: bcm2835-audio: Simplify card object management Takashi Iwai
2018-09-08 13:18 ` [PATCH 00/29] staging: bcm2835-audio: Cleanups and fixes Stefan Wahren
2018-09-08 16:21   ` Takashi Iwai
2018-09-08 17:00     ` Stefan Wahren
2018-09-08 17:16       ` Takashi Iwai
2018-09-10  9:12     ` Greg Kroah-Hartman
2018-09-10  9:16       ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180904155858.8001-6-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=eric@anholt.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=stefan.wahren@i2se.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).