Re: Re: ALC658 rev 0 - a problem with Rear Right

[Takashi Iwai <tiwai@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2906 bytes --]

At Mon, 7 Mar 2005 20:55:14 +0200,
Dan Aloni wrote:
> 
> On Mon, Mar 07, 2005 at 07:28:28PM +0100, Takashi Iwai wrote:
> > At Mon, 7 Mar 2005 20:24:14 +0200,
> > Dan Aloni wrote:
> > > 
> > > On Mon, Mar 07, 2005 at 06:13:44PM +0100, Takashi Iwai wrote:
> > > > At Mon, 7 Mar 2005 18:57:02 +0200,
> > > > Dan Aloni wrote:
> > > > > 
> > > > > On Mon, Mar 07, 2005 at 05:33:56PM +0100, Takashi Iwai wrote:
> > > > > > At Mon, 7 Mar 2005 17:39:21 +0200,
> > > > > > Dan Aloni wrote:
> > > > > > > 
> > > > > > > On Mon, Mar 07, 2005 at 02:10:14PM +0100, Clemens Ladisch wrote:
> > > > > > > > Dan Aloni wrote:
> > > > > > > > > The problem was introduced in the following changeset, which
> > > > > > > > > was supposed to cleanup things but accidently broke something.
> > > > > > > > >
> > > > > > > > > "[ALSA] Clean up and fix stereo mutes" :
> > > > > > > > 
> > > > > > > > Most AC'97 codecs have one mute bit for both channels, but some can
> > > > > > > > have one for each channel (that's called "stereo mute").
> > > > > > > > 
> > > > > > > > It seems the new driver enables the stereo mute bits, but then forgets
> > > > > > > > to unmute the second channel.
> > > > > > > > 
> > > > > > > > Does anything happen if you toggle both mute bits in alsamixer?
> > > > > > > 
> > > > > > > Using alsamixer I am unable to mute any channel by itself - it 
> > > > > > > always mutes or unmutes boths channels at the same time (I tried
> > > > > > > any possible combination of m M, < or >). However if I revert the
> > > > > > > changeset I am able to control each channel alone using < or > and
> > > > > > > there's no problem.
> > > > > > 
> > > > > > Hmm, are you sure that it's ALC658?  AFAIK, the codec doesn't  support
> > > > > > the stereo mutes (at least ALSA code doesn't enable it).
> > > > > 
> > > > > Unless the driver misidentifies the chipset, this is ALC658.
> > > > 
> > > > Oh yes, indeed.  It's strange that the older version did have the
> > > > stereo mute controls.
> > > > 
> > > > Could you compare the contents of
> > > > /proc/asound/card0/codec97#0/ac97#0-0+regs file between the older
> > > > ("working") and the latest versions?
> > > 
> > > I compared and saw no changes in the registers between the latest version 
> > > and my version with the reverted changeset.
> > 
> > Hmm, interesting.
> > 
> > Just to be sure (I'm back from the travel shortly ago, so might be
> > missing something):  the problem persists only on the surround right
> > channel?
> 
> Yes, only that channel.
>  
> > And you could mute on/off of "Surround" left/right channels via
> > alsamixer independently in the older version?
> 
> I can only mute both at the same time with the latest version, but I 
> can mute each channel alone with the working version.

OK.  Then please try the attached patch.  This will enable the stereo
mutes on the surround volume control again.


thanks,

Takashi

[-- Attachment #2: Type: text/plain, Size: 2319 bytes --]

Index: alsa-kernel/pci/ac97/ac97_codec.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/pci/ac97/ac97_codec.c,v
retrieving revision 1.165
diff -u -r1.165 ac97_codec.c
--- alsa-kernel/pci/ac97/ac97_codec.c	16 Feb 2005 18:25:30 -0000	1.165
+++ alsa-kernel/pci/ac97/ac97_codec.c	7 Mar 2005 19:11:27 -0000
@@ -1184,7 +1184,7 @@
 /*
  * create mute switch(es) for normal stereo controls
  */
-static int snd_ac97_cmute_new(snd_card_t *card, char *name, int reg, ac97_t *ac97)
+static int snd_ac97_cmute_new_stereo(snd_card_t *card, char *name, int reg, int check_stereo, ac97_t *ac97)
 {
 	snd_kcontrol_t *kctl;
 	int err;
@@ -1195,7 +1195,7 @@
 
 	mute_mask = 0x8000;
 	val = snd_ac97_read(ac97, reg);
-	if (ac97->flags & AC97_STEREO_MUTES) {
+	if (check_stereo || (ac97->flags & AC97_STEREO_MUTES)) {
 		/* check whether both mute bits work */
 		val1 = val | 0x8080;
 		snd_ac97_write(ac97, reg, val1);
@@ -1253,7 +1253,7 @@
 /*
  * create a mute-switch and a volume for normal stereo/mono controls
  */
-static int snd_ac97_cmix_new(snd_card_t *card, const char *pfx, int reg, ac97_t *ac97)
+static int snd_ac97_cmix_new_stereo(snd_card_t *card, const char *pfx, int reg, int check_stereo, ac97_t *ac97)
 {
 	int err;
 	char name[44];
@@ -1264,7 +1264,7 @@
 
 	if (snd_ac97_try_bit(ac97, reg, 15)) {
 		sprintf(name, "%s Switch", pfx);
-		if ((err = snd_ac97_cmute_new(card, name, reg, ac97)) < 0)
+		if ((err = snd_ac97_cmute_new_stereo(card, name, reg, check_stereo, ac97)) < 0)
 			return err;
 	}
 	check_volume_resolution(ac97, reg, &lo_max, &hi_max);
@@ -1276,6 +1276,8 @@
 	return 0;
 }
 
+#define snd_ac97_cmix_new(card, pfx, reg, ac97)	snd_ac97_cmix_new_stereo(card, pfx, reg, 0, ac97)
+#define snd_ac97_cmute_new(card, name, reg, ac97)	snd_ac97_cmute_new_stereo(card, name, reg, 0, ac97)
 
 static unsigned int snd_ac97_determine_spdif_rates(ac97_t *ac97);
 
@@ -1326,7 +1328,8 @@
 
 	/* build surround controls */
 	if (snd_ac97_try_volume_mix(ac97, AC97_SURROUND_MASTER)) {
-		if ((err = snd_ac97_cmix_new(card, "Surround Playback", AC97_SURROUND_MASTER, ac97)) < 0)
+		/* Surround Master (0x38) is with stereo mutes */
+		if ((err = snd_ac97_cmix_new_stereo(card, "Surround Playback", AC97_SURROUND_MASTER, 1, ac97)) < 0)
 			return err;
 	}