[PATCH 2/4] ALSA: usb-audio: Demote the severity of sticky GET_CUR

[Rong Zhang]

Currently, a mixer is disabled when its GET_CUR is sticky, causing
userspace to fall back to soft mixers, unless
QUIRK_FLAG_MIXER_GET_CUR_BROKEN is set. This leads to issues on some
wireless headphones with broken GET_CUR but effective SET_CUR, which use
poorly-performed lossy codecs and are prone to audible distortion at low
volume. They have to set the quirk flag to reeanble the mixer.

Considering that users can always opt into soft mixers if they need it,
i.e., when SET_CUR is stubbed, demote the severity of sticky GET_CUR by
marking GET_CUR as broken and only provide mixer values from the cache.
The mixer itself is still registered.

Signed-off-by: Rong Zhang <i@xxxxxxxx>
---
 sound/usb/mixer.c | 76 +++++++++++++++----------------------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 5de182181ede..3aa9a9c12557 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1237,21 +1237,27 @@ static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx)
 }
 
 /*
- * Additional checks for sticky mixers
+ * Additional checks for sticky GET_CUR
  *
- * Some devices' volume control mixers are sticky, which accept SET_CUR but
- * do absolutely nothing.
+ * Some devices' volume control mixers have sticky GET_CUR, which implies either
+ * stubbed SET_CUR or broken GET_CUR. For the former case, the mixer accepts
+ * SET_CUR but do absolutely nothing, so falling back to soft mixer is the only
+ * way to control the volume. For the latter case, the mixer has effective
+ * SET_CUR despite GET_CUR being constant, and the mixer is usable as long as we
+ * always provide mixer value from the ceche.
  *
  * Check the return values of GET_CUR with different SET_CUR values. Consider
- * the mixer as sticky if GET_CUR always returns a constant value.
+ * GET_CUR as sticky if GET_CUR always returns a constant value.
  *
- * Some devices have effective SET_CUR despite GET_CUR being constant. Do not
- * consider the mixer as sticky if a quirk flag indicates that.
+ * Unfortunately, we can't distinguish between stubbed SET_CUR and broken
+ * GET_CUR with simple read-back tests. Disabling the mixer regardless and
+ * forcing userspace to use soft mixer instead can lead to audible distortion at
+ * low volume on some wireless headphones, probably due to their poorly-
+ * performed lossy codec.
  *
- * Gate the registration of sticky mixers to prevent confusing userspace, so
- * that they won't cause ineffective volume control. However, for mixers with
- * effective SET_CUR but broken GET_CUR, the registration can continue normally
- * but further GET_CUR requests will be gated.
+ * Instead, mark GET_CUR as broken regardless and only provide mixer value from
+ * the cache. Users may opt into soft mixer in userspace audio stack if they
+ * need it.
  */
 static int check_sticky_volume_control(struct usb_mixer_elem_info *cval,
 				       int channel, int saved)
@@ -1271,24 +1277,13 @@ static int check_sticky_volume_control(struct usb_mixer_elem_info *cval,
 			return 0;
 	}
 
-	if (cval->head.mixer->chip->quirk_flags & QUIRK_FLAG_MIXER_GET_CUR_BROKEN) {
-		usb_audio_info(cval->head.mixer->chip,
-			       "%d:%d: broken mixer GET_CUR (%d/%d/%d => %d)\n",
-			       cval->head.id, mixer_ctrl_intf(cval->head.mixer),
-			       cval->min, cval->max, cval->res, saved);
-
-		cval->get_cur_broken = 1;
-		return -ENXIO;
-	}
-
-	usb_audio_err(cval->head.mixer->chip,
-		      "%d:%d: sticky mixer values (%d/%d/%d => %d), disabling\n",
-		      cval->head.id, mixer_ctrl_intf(cval->head.mixer),
-		      cval->min, cval->max, cval->res, saved);
 	usb_audio_info(cval->head.mixer->chip,
-		       "check MIXER_GET_CUR_BROKEN if you believe the mixer is non-sticky");
+		       "%d:%d: broken mixer GET_CUR (%d/%d/%d => %d)\n",
+		       cval->head.id, mixer_ctrl_intf(cval->head.mixer),
+		       cval->min, cval->max, cval->res, saved);
 
-	return -ENODEV;
+	cval->get_cur_broken = 1;
+	return -ENXIO;
 }
 
 /*
@@ -1385,8 +1380,6 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 				goto no_checks;
 
 			ret = check_sticky_volume_control(cval, minchn, saved);
-			if (ret == -ENODEV)
-				goto sticky;
 			if (ret)
 				goto no_checks;
 
@@ -1455,33 +1448,6 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval,
 	}
 
 	return 0;
-
-sticky:
-	/*
-	 * It makes no sense to restore the saved value for a sticky mixer,
-	 * since setting any value is a no-op.
-	 *
-	 * However, in some rare cases, SET_CUR is effective despite GET_CUR
-	 * always returns a constant value. These mixers are not sticky, but
-	 * there's no way to distinguish them. Without any additional
-	 * information, the best thing we can do is to set the mixer value to
-	 * the maximum before bailing out, so that a soft mixer can still reach
-	 * the maximum hardware volume if the mixer turns out to be non-sticky.
-	 * Meanwhile, all channels must be synchronized to prevent imbalance
-	 * volume.
-	 */
-	if (!cval->cmask) {
-		snd_usb_set_cur_mix_value(cval, 0, 0, cval->max);
-	} else {
-		idx = 0;
-		for (i = 0; i < MAX_CHANNELS; i++) {
-			if (cval->cmask & BIT(i)) {
-				snd_usb_set_cur_mix_value(cval, i + 1, idx, cval->max);
-				idx++;
-			}
-		}
-	}
-	return ret;
 }
 
 #define get_min_max(cval, def)	get_min_max_with_quirks(cval, def, NULL)

-- 
2.55.0