[alsa-devel] [PATCH 1/3] ASoC: cs4270: fix Master Capture Switch polarity
The control modifies the MUTE register, hence the polarity must be inverted.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk Cc: Timur Tabi timur@freescale.com --- sound/soc/codecs/cs4270.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c index 7fa09a3..3c34fe6 100644 --- a/sound/soc/codecs/cs4270.c +++ b/sound/soc/codecs/cs4270.c @@ -486,7 +486,7 @@ static const struct snd_kcontrol_new cs4270_snd_controls[] = { SOC_SINGLE("Zero Cross Switch", CS4270_TRANS, 5, 1, 0), SOC_SINGLE("Popguard Switch", CS4270_MODE, 0, 1, 1), SOC_SINGLE("Auto-Mute Switch", CS4270_MUTE, 5, 1, 0), - SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 0) + SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1) };
/*
This new control exports cs4270's DAC MUTE capabilities.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk Cc: Timur Tabi timur@freescale.com --- sound/soc/codecs/cs4270.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c index 3c34fe6..c5d6388 100644 --- a/sound/soc/codecs/cs4270.c +++ b/sound/soc/codecs/cs4270.c @@ -486,6 +486,7 @@ static const struct snd_kcontrol_new cs4270_snd_controls[] = { SOC_SINGLE("Zero Cross Switch", CS4270_TRANS, 5, 1, 0), SOC_SINGLE("Popguard Switch", CS4270_MODE, 0, 1, 1), SOC_SINGLE("Auto-Mute Switch", CS4270_MUTE, 5, 1, 0), + SOC_DOUBLE("Master Playback Switch", CS4270_MUTE, 0, 1, 1, 1), SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1) };
On Fri, Apr 24, 2009 at 03:00:26PM +0200, Daniel Mack wrote:
This new control exports cs4270's DAC MUTE capabilities.
The DAC mute is controled by cs4270_mute function - DAC mute is handles specially since many CODECs want to have mute synchronised with power down of the DAC. If cs4270 doesn't need this then remove the DAI operation too.
On Fri, Apr 24, 2009 at 02:17:38PM +0100, Mark Brown wrote:
On Fri, Apr 24, 2009 at 03:00:26PM +0200, Daniel Mack wrote:
This new control exports cs4270's DAC MUTE capabilities.
The DAC mute is controled by cs4270_mute function - DAC mute is handles specially since many CODECs want to have mute synchronised with power down of the DAC. If cs4270 doesn't need this then remove the DAI operation too.
Well, what I'm willing to implement is a way to manually mute the codec while it is running, independendly from other ways.
I've seen that dai function and untruly believed that those two ways could peacefully co-exist. But as it turns out, they currently influence each other, and a manually set mute will be overridden by the next stream start via the dai function. Which is bad.
I'll send a new patch for that, providing this feature in a slightly more complex fashion. However, I don't want to remove the other function. If the soc core decides to mute the codec, it should be able to do so.
Daniel
On Fri, Apr 24, 2009 at 03:52:16PM +0200, Daniel Mack wrote:
I'll send a new patch for that, providing this feature in a slightly more complex fashion. However, I don't want to remove the other function. If the soc core decides to mute the codec, it should be able to do so.
It's not the core that needs it, it's the DAC that normally needs the core to mute it (modulo random floating signals that it might parse) - if cs4270 is happy without the mute then the core doesn't care if it's not there.
On Fri, Apr 24, 2009 at 02:54:40PM +0100, Mark Brown wrote:
I'll send a new patch for that, providing this feature in a slightly more complex fashion. However, I don't want to remove the other function. If the soc core decides to mute the codec, it should be able to do so.
It's not the core that needs it, it's the DAC that normally needs the core to mute it (modulo random floating signals that it might parse) - if cs4270 is happy without the mute then the core doesn't care if it's not there.
This codec has pins which are usually connected to some external logic that can switch off aplifiers and the like when the code is in its mute state. Which is a nice feature: once you stop the ALSA output, everything behind the codec in the analog section is put into a power saving mode and is magically revived once ALSA starts using it again. That's why I want to leave it in there, even if the codecs does not necessarily need it.
Daniel
Add a macro for double controls with special callback functions.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk --- include/sound/soc.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index a40bc6f..db852b3 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -118,6 +118,14 @@ .info = snd_soc_info_volsw, \ .get = xhandler_get, .put = xhandler_put, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmax, xinvert) } +#define SOC_DOUBLE_EXT(xname, xreg, shift_left, shift_right, xmax, xinvert,\ + xhandler_get, xhandler_put) \ +{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ + .info = snd_soc_info_volsw, \ + .get = xhandler_get, .put = xhandler_put, \ + .private_value = (unsigned long)&(struct soc_mixer_control) \ + {.reg = xreg, .shift = shift_left, .rshift = shift_right, \ + .max = xmax, .invert = xinvert} } #define SOC_SINGLE_EXT_TLV(xname, xreg, xshift, xmax, xinvert,\ xhandler_get, xhandler_put, tlv_array) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
This adds a new control named 'Master Playback Switch' for cs4270 codecs. It is implemented using the new SOC_DOUBLE_EXT macro to catch the put function and store the information about manually set mute controls from userspace. When a manual mute is set, we don't want the soc core to un-mute the outputs.
Renamed cs4270_mute() to cs4270_dai_mute() to avoid confusion.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk Cc: Timur Tabi timur@freescale.com --- sound/soc/codecs/cs4270.c | 44 +++++++++++++++++++++++++++++++++++++++----- 1 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c index 3c34fe6..0f453a0 100644 --- a/sound/soc/codecs/cs4270.c +++ b/sound/soc/codecs/cs4270.c @@ -109,6 +109,7 @@ struct cs4270_private { unsigned int mclk; /* Input frequency of the MCLK pin */ unsigned int mode; /* The mode (I2S or left-justified) */ unsigned int slave_mode; + unsigned int manual_mute; };
/** @@ -453,7 +454,7 @@ static int cs4270_hw_params(struct snd_pcm_substream *substream, }
/** - * cs4270_mute - enable/disable the CS4270 external mute + * cs4270_dai_mute - enable/disable the CS4270 external mute * @dai: the SOC DAI * @mute: 0 = disable mute, 1 = enable mute * @@ -462,21 +463,52 @@ static int cs4270_hw_params(struct snd_pcm_substream *substream, * board does not have the MUTEA or MUTEB pins connected to such circuitry, * then this function will do nothing. */ -static int cs4270_mute(struct snd_soc_dai *dai, int mute) +static int cs4270_dai_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_codec *codec = dai->codec; + struct cs4270_private *cs4270 = codec->private_data; int reg6;
reg6 = snd_soc_read(codec, CS4270_MUTE);
if (mute) reg6 |= CS4270_MUTE_DAC_A | CS4270_MUTE_DAC_B; - else + else { reg6 &= ~(CS4270_MUTE_DAC_A | CS4270_MUTE_DAC_B); + reg6 |= cs4270->manual_mute; + }
return snd_soc_write(codec, CS4270_MUTE, reg6); }
+/** + * cs4270_soc_put_mute - put callback for the 'Master Playback switch' + * alsa control. + * @kcontrol: mixer control + * @ucontrol: control element information + * + * This function basically passes the arguments on to the generic + * snd_soc_put_volsw() function and saves the mute information in + * our private data structure. This is because we want to prevent + * cs4270_dai_mute() neglecting the user's decision to manually + * mute the codec's output. + * + * Returns 0 for success. + */ +static int cs4270_soc_put_mute(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); + struct cs4270_private *cs4270 = codec->private_data; + int left = !ucontrol->value.integer.value[0]; + int right = !ucontrol->value.integer.value[1]; + + cs4270->manual_mute = (left ? CS4270_MUTE_DAC_A : 0) | + (right ? CS4270_MUTE_DAC_B : 0); + + return snd_soc_put_volsw(kcontrol, ucontrol); +} + /* A list of non-DAPM controls that the CS4270 supports */ static const struct snd_kcontrol_new cs4270_snd_controls[] = { SOC_DOUBLE_R("Master Playback Volume", @@ -486,7 +518,9 @@ static const struct snd_kcontrol_new cs4270_snd_controls[] = { SOC_SINGLE("Zero Cross Switch", CS4270_TRANS, 5, 1, 0), SOC_SINGLE("Popguard Switch", CS4270_MODE, 0, 1, 1), SOC_SINGLE("Auto-Mute Switch", CS4270_MUTE, 5, 1, 0), - SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1) + SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1), + SOC_DOUBLE_EXT("Master Playback Switch", CS4270_MUTE, 0, 1, 1, 1, + snd_soc_get_volsw, cs4270_soc_put_mute) };
/* @@ -506,7 +540,7 @@ static struct snd_soc_dai_ops cs4270_dai_ops = { .hw_params = cs4270_hw_params, .set_sysclk = cs4270_set_dai_sysclk, .set_fmt = cs4270_set_dai_fmt, - .digital_mute = cs4270_mute, + .digital_mute = cs4270_dai_mute, };
struct snd_soc_dai cs4270_dai = {
On Fri, Apr 24, 2009 at 9:37 AM, Daniel Mack daniel@caiaq.de wrote:
This adds a new control named 'Master Playback Switch' for cs4270 codecs. It is implemented using the new SOC_DOUBLE_EXT macro to catch the put function and store the information about manually set mute controls from userspace. When a manual mute is set, we don't want the soc core to un-mute the outputs.
Renamed cs4270_mute() to cs4270_dai_mute() to avoid confusion.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk Cc: Timur Tabi timur@freescale.com
Acked-by: Timur Tabi timur@freescale.com
However, ...
@@ -486,7 +518,9 @@ static const struct snd_kcontrol_new cs4270_snd_controls[] = { SOC_SINGLE("Zero Cross Switch", CS4270_TRANS, 5, 1, 0), SOC_SINGLE("Popguard Switch", CS4270_MODE, 0, 1, 1), SOC_SINGLE("Auto-Mute Switch", CS4270_MUTE, 5, 1, 0),
- SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1)
- SOC_DOUBLE("Master Capture Switch", CS4270_MUTE, 3, 4, 1, 1),
- SOC_DOUBLE_EXT("Master Playback Switch", CS4270_MUTE, 0, 1, 1, 1,
- snd_soc_get_volsw, cs4270_soc_put_mute)
};
Mark, could you add a comma at the end of "cs4270_soc_put_mute)" so that any future patches won't need to delete any lines?
Daniel Mack wrote:
The control modifies the MUTE register, hence the polarity must be inverted.
I have a question. In alsamixer, the capture switch looks either like this:
------
< Master >
or like this:
L R CAPTUR
< Master >
What exactly does this mean? Which one is supposed to mean "muted"? And how come even though this switch is a SOC_DOUBLE, I cannot control the left and right channel independently with the < and > keys, and the "M" key doesn't toggle it either.
On Fri, Apr 24, 2009 at 10:45:29AM -0500, Timur Tabi wrote:
Daniel Mack wrote:
The control modifies the MUTE register, hence the polarity must be inverted.
I have a question. In alsamixer, the capture switch looks either like this:
------ < Master >or like this:
L R CAPTUR < Master >What exactly does this mean? Which one is supposed to mean "muted"?
The switch's name is 'Master Capture Switch' which means to me that it is a thing that switches the capture on and off, and hence, it should be set to 'on' for normal operation. If it was meant to mute the input when switched on, the name should be 'Master Caputure Mute Switch' or something, right?
And how come even though this switch is a SOC_DOUBLE, I cannot control the left and right channel independently with the < and > keys, and the "M" key doesn't toggle it either.
Don't know about the alsamixer tool, but with amixer, you can set the channels individually with the following syntax:
$ amixer cset numid=8 on,off numid=8,iface=MIXER,name='Master Playback Switch' ; type=BOOLEAN,access=rw------,values=2 : values=on,off
Daniel
Daniel Mack wrote:
The switch's name is 'Master Capture Switch' which means to me that it is a thing that switches the capture on and off, and hence, it should be set to 'on' for normal operation. If it was meant to mute the input when switched on, the name should be 'Master Caputure Mute Switch' or something, right?
Beats me. I'm the one asking questions! But you still haven't told me the difference between "------" and "L R". Does "------" mean that capture should be muted, because it's been switched off?
Don't know about the alsamixer tool, but with amixer, you can set the channels individually with the following syntax:
$ amixer cset numid=8 on,off numid=8,iface=MIXER,name='Master Playback Switch' ; type=BOOLEAN,access=rw------,values=2 : values=on,off
I guess it's an alsamixer bug.
On Fri, Apr 24, 2009 at 11:13:08AM -0500, Timur Tabi wrote:
The switch's name is 'Master Capture Switch' which means to me that it is a thing that switches the capture on and off, and hence, it should be set to 'on' for normal operation. If it was meant to mute the input when switched on, the name should be 'Master Caputure Mute Switch' or something, right?
Beats me. I'm the one asking questions!
:)
But you still haven't told me the difference between "------" and "L R". Does "------" mean that capture should be muted, because it's been switched off?
Sorry, don't know. amixer is my reference and in the high-level software using that interface, things are implemented similarly. All I can say is that the control does not expose any information about whether it's high or low active, so all that needs to be taken care for is that name and function match.
Daniel
On Fri, Apr 24, 2009 at 10:45:29AM -0500, Timur Tabi wrote:
Daniel Mack wrote:
The control modifies the MUTE register, hence the polarity must be inverted.
L R CAPTUR < Master >
What exactly does this mean? Which one is supposed to mean "muted"?
Capture is enabled in the picture above; often when the alsamixer UI is confusing amixer is a bit clearer about what's going on.
And how come even though this switch is a SOC_DOUBLE, I cannot control the left and right channel independently with the < and > keys, and the "M" key doesn't toggle it either.
This is a property of capture controls. I'm not entirely sure why, CCing in Takashi who should be able to offer more knowledgable comment.
At Sun, 26 Apr 2009 10:49:17 +0100, Mark Brown wrote:
On Fri, Apr 24, 2009 at 10:45:29AM -0500, Timur Tabi wrote:
Daniel Mack wrote:
The control modifies the MUTE register, hence the polarity must be inverted.
L R CAPTUR < Master >What exactly does this mean? Which one is supposed to mean "muted"?
Capture is enabled in the picture above; often when the alsamixer UI is confusing amixer is a bit clearer about what's going on.
The "mute" is only for "* Playback Switch" mixer controls. In this case, if you have "Master Playback Switch", it's handled as the mute switch in the mixer interface. If you don't have such a control element, there is no "mute" for "Master" mixer element.
OTOH, "* Capture Switch" is handled as "capture" switch, which is indicated as "CAPTURE" (and "L" & "R") in the above. Also, if the driver supports rather "Capture Source" enum instead of "XXX Capture Switch", its list items are converted to multiple exclusive "capture" switches.
And how come even though this switch is a SOC_DOUBLE, I cannot control the left and right channel independently with the < and > keys, and the "M" key doesn't toggle it either.
This is a property of capture controls. I'm not entirely sure why, CCing in Takashi who should be able to offer more knowledgable comment.
The keys you mentioned above are all for playback mute controls, thus irrelevant with the capture switch. User space key to toggle the capture switch.
Takashi
On Sun, Apr 26, 2009 at 01:08:22PM +0200, Takashi Iwai wrote:
OTOH, "* Capture Switch" is handled as "capture" switch, which is indicated as "CAPTURE" (and "L" & "R") in the above. Also, if the
Right; however the effect of a capture switch is pretty much equivalent to mute for a playback control.
The keys you mentioned above are all for playback mute controls, thus irrelevant with the capture switch. User space key to toggle the capture switch.
That doesn't quite answer Timur's question - the question was more why capture switches are being presented to the user as mono controls even when the two channels can be switched independantly.
At Sun, 26 Apr 2009 12:21:37 +0100, Mark Brown wrote:
On Sun, Apr 26, 2009 at 01:08:22PM +0200, Takashi Iwai wrote:
OTOH, "* Capture Switch" is handled as "capture" switch, which is indicated as "CAPTURE" (and "L" & "R") in the above. Also, if the
Right; however the effect of a capture switch is pretty much equivalent to mute for a playback control.
Although the switch is the inverted direction of the mute.
The keys you mentioned above are all for playback mute controls, thus irrelevant with the capture switch. User space key to toggle the capture switch.
That doesn't quite answer Timur's question - the question was more why capture switches are being presented to the user as mono controls even when the two channels can be switched independantly.
No, they are handled actually as stereo. That's why "L" and "R" are shown in alsamixer. They can be toggled independently via ";" and "'" keys. It's just not written in the man page.
Before complaining, patches are appreciated, of course :)
Takashi
On Sun, Apr 26, 2009 at 01:08:22PM +0200, Takashi Iwai wrote:
Capture is enabled in the picture above; often when the alsamixer UI is confusing amixer is a bit clearer about what's going on.
The "mute" is only for "* Playback Switch" mixer controls. In this case, if you have "Master Playback Switch", it's handled as the mute switch in the mixer interface. If you don't have such a control element, there is no "mute" for "Master" mixer element.
So but what about the polarity then? 'Master Playback Switch' == 'on' means _not_ muted, right?
Daniel
At Sun, 26 Apr 2009 13:30:48 +0200, Daniel Mack wrote:
On Sun, Apr 26, 2009 at 01:08:22PM +0200, Takashi Iwai wrote:
Capture is enabled in the picture above; often when the alsamixer UI is confusing amixer is a bit clearer about what's going on.
The "mute" is only for "* Playback Switch" mixer controls. In this case, if you have "Master Playback Switch", it's handled as the mute switch in the mixer interface. If you don't have such a control element, there is no "mute" for "Master" mixer element.
So but what about the polarity then? 'Master Playback Switch' == 'on' means _not_ muted, right?
Right. The switch on = the sound comes out.
Takashi
On Sun, Apr 26, 2009 at 06:52:50PM +0200, Takashi Iwai wrote:
The "mute" is only for "* Playback Switch" mixer controls. In this case, if you have "Master Playback Switch", it's handled as the mute switch in the mixer interface. If you don't have such a control element, there is no "mute" for "Master" mixer element.
So but what about the polarity then? 'Master Playback Switch' == 'on' means _not_ muted, right?
Right. The switch on = the sound comes out.
Ok to take my patch then?
Daniel
On Fri, Apr 24, 2009 at 8:00 AM, Daniel Mack daniel@caiaq.de wrote:
The control modifies the MUTE register, hence the polarity must be inverted.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Mark Brown broonie@sirena.org.uk Cc: Timur Tabi timur@freescale.com
Acked-by: Timur Tabi timur@freescale.com
participants (4)
-
Daniel Mack -
Mark Brown -
Takashi Iwai -
Timur Tabi