[alsa-devel] [regression] probably in mixer.c code

Takashi Iwai tiwai at suse.de
Tue Nov 8 15:54:04 CET 2011


At Tue, 08 Nov 2011 15:32:10 +0100,
Alexey Fisher wrote:
> 
> Am 08.11.2011 14:09, schrieb Takashi Iwai:
> > At Tue, 08 Nov 2011 14:05:32 +0100,
> > Takashi Iwai wrote:
> >>
> >> At Tue, 08 Nov 2011 13:39:38 +0100,
> >> Alexey Fisher wrote:
> >>>
> >>> Am 08.11.2011 13:11, schrieb Takashi Iwai:
> >>>> At Tue, 08 Nov 2011 12:25:06 +0100,
> >>>> Alexey Fisher wrote:
> >>>>>
> >>>>> Am 08.11.2011 10:59, schrieb Takashi Iwai:
> >>>>>> At Tue, 08 Nov 2011 09:49:13 +0100,
> >>>>>> Alexey Fisher wrote:
> >>>>>>>
> >>>>>>> Hallo all, Takashi,
> >>>>>>>
> >>>>>>> there is one regression in current linux main tree:
> >>>>>>>
> >>>>>>> cval->res quirks do not work any more. See usb/mixer.c:1150
> >>>>>>> Are there was some work in this area or may be some one bisected this
> >>>>>>> bug down?
> >>>>>>
> >>>>>> Are you referring to the device 046d:09a4, which sets the mixer range
> >>>>>> manually?  If yes, does the patch below work?
> >>>>>>
> >>>>>
> >>>>>
> >>>>> No, i referring to devices 046d:0991, 046d:081d, i don't have access to
> >>>>> 046d:09a4.
> >>>>>
> >>>>> This part is broken for me:
> >>>>>            case USB_ID(0x046d, 0x0991):
> >>>>>            /* Most audio usb devices lie about volume resolution.
> >>>>>             * Most Logitech webcams have res = 384.
> >>>>>             * Proboly there is some logitech magic behind this number --fishor
> >>>>>             */
> >>>>>                    if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
> >>>>>                            snd_printk(KERN_INFO
> >>>>>                                    "set resolution quirk: cval->res = 384\n");
> >>>>>                            cval->res = 384;
> >>>>>                    }
> >>>>>                    break;
> >>>>
> >>>> OK, so _what_ is actually broken?  Do you see the message but
> >>>> cval->res is overridden later?
> >>>
> >>> yes.
> >>
> >> To which value?
> >
> > Also you should have got a message like "0:1: cannot get min/max
> > values for ..." when built with CONFIG_SND_DEBUG.  Check whether you
> > get it actually.
> 
> It sets the res value to 1.
> I get fallowing message after i plug in the webcam:
> 
> [  132.878150] usb 1-3.2: new high-speed USB device number 8 using ehci_hcd
> [  133.087255] usb 1-3.2: New USB device found, idVendor=046d, 
> idProduct=0991
> [  133.087259] usb 1-3.2: New USB device strings: Mfr=0, Product=0, 
> SerialNumber=2
> [  133.087261] usb 1-3.2: SerialNumber: 9671DCEE
> [  133.088428] uvcvideo: Found UVC 1.00 device <unnamed> (046d:0991)
> [  133.122400] input: UVC Camera (046d:0991) as 
> /devices/pci0000:00/0000:00:1a.7/usb1/1-3/1-3.2/1-3.2:1.0/input/input15
> [  133.488506] ALSA mixer.c:817 5:2: cannot get min/max values for 
> control 2 (id 5)
> [  133.488524] ALSA mixer.c:1168 set resolution quirk: cval->res = 384
> [  133.488527] ALSA mixer.c:817 5:2: cannot get min/max values for 
> control 2 (id 5)
> [  133.492187] ALSA mixer.c:817 5:2: cannot get min/max values for 
> control 2 (id 5)
> 
> 
> With amixer i get this:
> amixer -c U0x46d0x991
> Simple mixer control 'Mic',0
>    Capabilities: cvolume cvolume-joined cswitch cswitch-joined penum
>    Capture channels: Mono
>    Limits: Capture 0 - 3072
>    Mono: Capture 2749 [89%] [28.73dB] [on]
> 
> 
> But Limits should be 0-7 instead of 0-3072

OK, then the problem is that the first few reads of min/max values
failed, but cval->res quirk is applied only for the first read.

Does the patch below fix?


Takashi

---
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 60f65ac..c5444e0 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -765,10 +765,60 @@ static void usb_mixer_elem_free(struct snd_kcontrol *kctl)
  * interface to ALSA control for feature/mixer units
  */
 
+/* volume control quirks */
+static void volume_control_quirks(struct usb_mixer_elem_info *cval,
+				  struct snd_kcontrol *kctl)
+{
+	switch (cval->mixer->chip->usb_id) {
+	case USB_ID(0x0471, 0x0101):
+	case USB_ID(0x0471, 0x0104):
+	case USB_ID(0x0471, 0x0105):
+	case USB_ID(0x0672, 0x1041):
+	/* quirk for UDA1321/N101.
+	 * note that detection between firmware 2.1.1.7 (N101)
+	 * and later 2.1.1.21 is not very clear from datasheets.
+	 * I hope that the min value is -15360 for newer firmware --jk
+	 */
+		if (!strcmp(kctl->id.name, "PCM Playback Volume") &&
+		    cval->min == -15616) {
+			snd_printk(KERN_INFO
+				 "set volume quirk for UDA1321/N101 chip\n");
+			cval->max = -256;
+		}
+		break;
+
+	case USB_ID(0x046d, 0x09a4):
+		if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
+			snd_printk(KERN_INFO
+				"set volume quirk for QuickCam E3500\n");
+			cval->min = 6080;
+			cval->max = 8768;
+			cval->res = 192;
+		}
+		break;
+
+	case USB_ID(0x046d, 0x0808):
+	case USB_ID(0x046d, 0x0809):
+	case USB_ID(0x046d, 0x0991):
+	/* Most audio usb devices lie about volume resolution.
+	 * Most Logitech webcams have res = 384.
+	 * Proboly there is some logitech magic behind this number --fishor
+	 */
+		if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
+			snd_printk(KERN_INFO
+				"set resolution quirk: cval->res = 384\n");
+			cval->res = 384;
+		}
+		break;
+
+	}
+}
+
 /*
  * retrieve the minimum and maximum values for the specified control
  */
-static int get_min_max(struct usb_mixer_elem_info *cval, int default_min)
+static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
+				   int default_min, struct snd_kcontrol *kctl)
 {
 	/* for failsafe */
 	cval->min = default_min;
@@ -844,6 +894,9 @@ static int get_min_max(struct usb_mixer_elem_info *cval, int default_min)
 		cval->initialized = 1;
 	}
 
+	if (kctl)
+		volume_control_quirks(cval, kctl);
+
 	/* USB descriptions contain the dB scale in 1/256 dB unit
 	 * while ALSA TLV contains in 1/100 dB unit
 	 */
@@ -864,6 +917,7 @@ static int get_min_max(struct usb_mixer_elem_info *cval, int default_min)
 	return 0;
 }
 
+#define get_min_max(cval, def)	get_min_max_with_quirks(cval, def, NULL)
 
 /* get a feature/mixer unit info */
 static int mixer_ctl_feature_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo)
@@ -882,7 +936,7 @@ static int mixer_ctl_feature_info(struct snd_kcontrol *kcontrol, struct snd_ctl_
 		uinfo->value.integer.max = 1;
 	} else {
 		if (!cval->initialized) {
-			get_min_max(cval, 0);
+			get_min_max_with_quirks(cval, 0, kcontrol);
 			if (cval->initialized && cval->dBmin >= cval->dBmax) {
 				kcontrol->vd[0].access &= 
 					~(SNDRV_CTL_ELEM_ACCESS_TLV_READ |
@@ -1045,9 +1099,6 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 		cval->ch_readonly = readonly_mask;
 	}
 
-	/* get min/max values */
-	get_min_max(cval, 0);
-
 	/* if all channels in the mask are marked read-only, make the control
 	 * read-only. set_cur_mix_value() will check the mask again and won't
 	 * issue write commands to read-only channels. */
@@ -1069,6 +1120,9 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 		len = snd_usb_copy_string_desc(state, nameid,
 				kctl->id.name, sizeof(kctl->id.name));
 
+	/* get min/max values */
+	get_min_max_with_quirks(cval, 0, kctl);
+
 	switch (control) {
 	case UAC_FU_MUTE:
 	case UAC_FU_VOLUME:
@@ -1118,51 +1172,6 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
 		break;
 	}
 
-	/* volume control quirks */
-	switch (state->chip->usb_id) {
-	case USB_ID(0x0471, 0x0101):
-	case USB_ID(0x0471, 0x0104):
-	case USB_ID(0x0471, 0x0105):
-	case USB_ID(0x0672, 0x1041):
-	/* quirk for UDA1321/N101.
-	 * note that detection between firmware 2.1.1.7 (N101)
-	 * and later 2.1.1.21 is not very clear from datasheets.
-	 * I hope that the min value is -15360 for newer firmware --jk
-	 */
-		if (!strcmp(kctl->id.name, "PCM Playback Volume") &&
-		    cval->min == -15616) {
-			snd_printk(KERN_INFO
-				 "set volume quirk for UDA1321/N101 chip\n");
-			cval->max = -256;
-		}
-		break;
-
-	case USB_ID(0x046d, 0x09a4):
-		if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
-			snd_printk(KERN_INFO
-				"set volume quirk for QuickCam E3500\n");
-			cval->min = 6080;
-			cval->max = 8768;
-			cval->res = 192;
-		}
-		break;
-
-	case USB_ID(0x046d, 0x0808):
-	case USB_ID(0x046d, 0x0809):
-	case USB_ID(0x046d, 0x0991):
-	/* Most audio usb devices lie about volume resolution.
-	 * Most Logitech webcams have res = 384.
-	 * Proboly there is some logitech magic behind this number --fishor
-	 */
-		if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
-			snd_printk(KERN_INFO
-				"set resolution quirk: cval->res = 384\n");
-			cval->res = 384;
-		}
-		break;
-
-	}
-
 	range = (cval->max - cval->min) / cval->res;
 	/* Are there devices with volume range more than 255? I use a bit more
 	 * to be sure. 384 is a resolution magic number found on Logitech


More information about the Alsa-devel mailing list