Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Alexander Niemeyer <adventureFAN@gmx.de>
To: Rong Zhang <i@rong.moe>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>
Cc: Takashi Iwai <tiwai@suse.de>,
	linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
Date: Tue, 18 Aug 2026 16:41:39 +0200	[thread overview]
Message-ID: <0b04f4ea-03bf-4fad-a467-2b378331137f@gmx.de> (raw)
In-Reply-To: <6acbf6556133790ad093c5c4bb4a451526007e05.camel@rong.moe>

Hi Rong,

I think we found the reason for the different behavior.

I reproduced the snd-usb-audio initialization sequence step by step with 
direct libusb UAC1 control transfers and isolated the problem to SET_RES 
on the *Mic Capture Volume control (Feature Unit 3)*.

A fresh-device control test looks like this:

Mic GET_RES = 256
no SET_RES
Playback:
GET_CUR = -3840 (-15 dB)
SET_CUR = -2048 (-8 dB)
GET_CUR changes to -2048 after 68.0 ms
Result: PASS

After another power cycle, I repeated the same test but issued just 
*one* SET_RES request to the Mic Feature Unit first:

Mic GET_RES before = 256
Mic SET_RES(128) = success
Mic GET_RES after = 256
Playback:
GET_CUR = -3840 (-15 dB)
SET_CUR = -2048 (-8 dB)
GET_CUR remains -3840 for more than 1200 ms
Result: FAIL

So a single successful |SET_RES(128)| on Feature Unit 3 is sufficient to 
make subsequent |SET_CUR| requests to the Playback Volume control on 
Feature Unit 2 ineffective.

I also tested the complete Mic SET_RES sequence used by snd-usb-audio:

SET_RES 128
SET_RES 64
SET_RES 32
SET_RES 16
SET_RES 8
SET_RES 4
SET_RES 2
SET_RES 1

All requests return success, while GET_RES remains 256. After that 
sequence, Playback SET_CUR also remains ineffective for more than 1200 ms.

Interestingly, the Mic control itself still works after this. In an 
ALSA-like Mic probe I could successfully change Mic Volume from 0 dB to 
-64 dB and then +1 dB, with GET_CUR reflecting those changes essentially 
immediately (~0.3 ms). Playback remained broken afterwards.

I also checked whether SET_RES on the Playback Feature Unit itself 
causes the problem. It does not:

Playback GET_RES = 256
SET_RES 128 -> 64 -> 32 -> 16 -> 8 -> 4 -> 2 -> 1
GET_RES still = 256
Playback SET_CUR(-8 dB)
GET_CUR changes successfully after 87.7 ms

So the problematic operation appears specifically to be *SET_RES on the 
Mic Feature Unit affecting the Playback Feature Unit*.

I also clarified the separate advertised-minimum issue:

Playback SET_CUR(-64 dB):
no change after >1200 ms
followed by SET_CUR(-8 dB):
works normally after 54.9 ms
Playback SET_CUR(-63 dB):
works after 75.8 ms
followed by SET_CUR(-8 dB):
works after 43.6 ms

Therefore the broken -64 dB endpoint does not leave the device in the 
broken state; it is a separate issue. -63 dB works normally.

I also captured usbmon/pcapng traces for both a working direct-libusb 
SET_CUR sequence and the failing snd-usb-audio initialization, so I can 
send those as well if they are useful.

This also seems to explain why the sticky-check changes did not help: by 
the time snd-usb-audio reaches the Playback Volume sticky check, the 
earlier Mic SET_RES sanity test has already put the device into the 
state where Playback SET_CUR no longer takes effect.

Let me know if you would like me to test a patch or capture any 
additional traces.

Thanks,
Alexander


Am 16.08.2026 um 17:08 schrieb Rong Zhang:
> Hi Alexander,
>
> On Sun, 2026-08-16 at 16:09 +0200, Alexander Niemeyer wrote:
>> Hi Rong,
>>
>> Sure. The libusb tests were direct USB Audio Class 1 control transfers
>> to the headset using libusb/PyUSB, not ALSA mixer operations.
>>
>> I accessed Feature Unit 2 on AudioControl interface 0, master channel 0,
>> with the UAC1 Volume control selector:
>>
>> wValue = 0x0200 /* Volume control, master channel */
>> wIndex = 0x0200 /* Feature Unit 2, interface 0 */
>>
>> I used the standard class-specific requests directly, including GET_CUR,
>> GET_MIN, GET_MAX, GET_RES and SET_CUR, with signed 16-bit little-endian
>> volume values in 1/256 dB units.
>>
>> The device reported:
>>
>> GET_CUR: 0 ( 0 dB in that test)
>> GET_MIN: -16384 (-64 dB)
>> GET_MAX: 0 ( 0 dB)
>> GET_RES: 256 ( 1 dB)
>>
>> For the timing tests I issued SET_CUR for a target value and then
>> repeatedly queried GET_CUR until the value changed or the timeout expired.
>>
>> Valid values became visible after roughly:
>>
>> -1 dB ~81 ms
>> -2 dB ~52 ms
>> -4 dB ~47 ms
>> -8 dB ~47 ms
>> -16 dB ~52 ms
>> -32 dB ~47 ms
>>
>> The advertised -64 dB minimum behaved differently: SET_CUR returned
>> successfully, but GET_CUR did not change even after 1000 ms.
>>
>> To access the AudioControl interface with libusb, I unbound the
>> AudioControl interface from snd-usb-audio for the duration of the test.
>>
>> I did not intentionally open a playback stream during those libusb
>> tests. Because the AudioControl interface had been unbound from
>> snd-usb-audio, I also do not believe there was an active ALSA playback
>> stream at that point.
> Thanks for the information.
>
> Unfortunately, I still don't exactly see why the device behaved
> differently when GET_CUR/SET_CUR requests were sent from snd-usb-audio
> compared to your libusb tests.
>
> snd-usb-audio also tries SET_RES to test the sanity of GET_RES. Could you
> test if it breaks your device's GET_CUR?
>
> Maybe comparing them with usbmon can show some clues. You can use
> Wireshark to sniff /dev/usbmon*.
>
> Hint: a Thunderbolt port usually corresponds to a dedicated USB root hub.
> If you have one, plug the device to it to get pure usbmon trace results
> with no noisy URBs from other devices.
>
> Thanks,
> Rong
>
>> If the open-stream state is important, I can repeat the experiment
>> specifically controlling for playback-stream-open versus
>> playback-stream-closed.
>>
>> Thanks,
>> Alexander
>>
>> Am 16.08.2026 um 15:50 schrieb Rong Zhang:
>>> Hi Alexander,
>>>
>>> On Sun, 2026-08-16 at 07:14 +0200, Alexander Niemeyer wrote:
>>>> Hi Rong,
>>>>
>>>> I tested the sticky-check part of your patch on the Logitech PRO X
>>>> Wireless (046d:0aba) on Fedora 44, kernel 7.1.8-200.fc44.x86_64.
>>>>
>>>> Since your patch is based on a newer tree, I used a minimal backport of
>>>> the new ~16-value / 10 ms sticky-check logic to the 7.1.8 code. The
>>>> GET_CUR-broken handling from the newer tree was not included; GET_CUR
>>>> itself succeeds on this device.
>>>>
>>>> Unfortunately, the playback control is still classified as sticky:
>>>>
>>>> 2:0: sticky mixer values (-16384/0/256 => -3840), disabling
>>>>
>>>> I then instrumented the check and tried an additional diagnostic: after
>>>> every successful SET_CUR, wait 100 ms and perform another GET_CUR before
>>>> issuing the next SET_CUR.
>>>>
>>>> For the playback volume, the saved value was -3840 and GET_CUR remained
>>>> at -3840 for every tested value, even after 100 ms, for example:
>>>>
>>>> test=-15104 immediate=-3840 after100ms=-3840
>>>> test=-13824 immediate=-3840 after100ms=-3840
>>>> test=-3584 immediate=-3840 after100ms=-3840
>>>> test=-2304 immediate=-3840 after100ms=-3840
>>>> test=-1024 immediate=-3840 after100ms=-3840
>>>> test=0 immediate=-3840 after100ms=-3840
>>>>
>>>> So in this case the issue does not appear to be simply that the
>>>> accumulated 10 ms sleeps are too short. During the probe-time sticky
>>>> check, SET_CUR succeeds but GET_CUR for the playback control remains
>>>> unchanged even when each SET_CUR is given 100 ms before the next one.
>>>>
>>>> This differs from my previous direct libusb tests with the AudioControl
>>>> interface unbound, where valid SET_CUR values became visible through
>>>> GET_CUR after roughly 47–81 ms.
>>> Really interesting. Maybe the mixer changes its value only when there is
>>> an opened playback stream.
>>>
>>> Could you clarify your "libusb tests"?
>>>
>>> Thanks,
>>> Rong
>>>
>>>> The first debug line I saw with |saved=0| was from the Mic Capture
>>>> Volume control; that control changed immediately and returned as
>>>> non-sticky. The sequence above with |saved=-3840| is the problematic PCM
>>>> Playback Volume control.
>>>>
>>>> I'd be happy to test another version or run additional diagnostics if
>>>> useful.
>>>>
>>>> Best regards,
>>>> Alexander
>>>>
>>>>
>>>> Am 15.08.2026 um 23:47 schrieb Rong Zhang:
>>>>> Some mixers are asynchronous, and some have broken min/max. They are
>>>>> mistakenly considered sticky due to how the check is implemented.
>>>>>
>>>>> Check sticky mixers more precisely by checking approximately 16 values
>>>>> and adding a msleep(10) between each check, so that asynchronous mixers
>>>>> have enough time to change the value and mixers with broken min/max are
>>>>> checked properly. Additionally, mark GET_CUR as broken when
>>>>> get_cur_mix_raw() fails, instead of returning successfully.
>>>>>
>>>>> Reported-by: Alexander Niemeyer<adventureFAN@gmx.de>
>>>>> Closes:https://lore.kernel.org/r/6262cbbd-d1f2-4c9d-a1c7-9c5d12636f4b@gmx.de
>>>>> Signed-off-by: Rong Zhang<i@rong.moe>
>>>>> ---
>>>>>     sound/usb/mixer.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
>>>>>     1 file changed, 44 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
>>>>> index 703c118f9d4e..3d0f97730a06 100644
>>>>> --- a/sound/usb/mixer.c
>>>>> +++ b/sound/usb/mixer.c
>>>>> @@ -1256,22 +1256,59 @@ static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx)
>>>>>     static int check_sticky_volume_control(struct usb_mixer_elem_info *cval,
>>>>>     				       int channel, int saved)
>>>>>     {
>>>>> -	int sticky_test_values[] = { cval->min, cval->max };
>>>>> -	int test, check, i;
>>>>> +	int test, check, res;
>>>>> +
>>>>> +	/*
>>>>> +	 * Check approximately 16 values (15 intervals).
>>>>> +	 * If the resolution is not fine enough, check fewer values.
>>>>> +	 */
>>>>> +	res = DIV_ROUND_UP(cval->max - cval->min, 15);
>>>>> +	res = res ? roundup(res, cval->res) : cval->res;
>>>>> +
>>>>> +	/*
>>>>> +	 * If (cval->max - cval->min) is not a multiple of cval->res, we still
>>>>> +	 * want to test cval->max anyway.
>>>>> +	 */
>>>>> +	for (test = cval->min; test < cval->max + res; test += res) {
>>>>> +		if (test > cval->max)
>>>>> +			test = cval->max;
>>>>>     
>>>>> -	for (i = 0; i < ARRAY_SIZE(sticky_test_values); i++) {
>>>>> -		test = sticky_test_values[i];
>>>>>     		if (test == saved)
>>>>>     			continue;
>>>>>     
>>>>>     		/* Assume non-sticky on failure. */
>>>>> -		if (snd_usb_set_cur_mix_value(cval, channel, 0, test) ||
>>>>> -		    get_cur_mix_raw(cval, channel, &check) ||
>>>>> -		    check != saved) /* SET_CUR effective, non-sticky. */
>>>>> +		if (snd_usb_set_cur_mix_value(cval, channel, 0, test))
>>>>> +			return 0;
>>>>> +
>>>>> +		if (get_cur_mix_raw(cval, channel, &check))
>>>>> +			goto get_cur_broken;
>>>>> +		if (check != saved) /* SET_CUR effective, non-sticky. */
>>>>>     			return 0;
>>>>> +
>>>>> +		/*
>>>>> +		 * Leave some time for asynchronous mixers to change the value.
>>>>> +		 *
>>>>> +		 * Note that there is no need to wait between SET_CUR and
>>>>> +		 * GET_CUR, as we don't care whether the GET_CUR value matches
>>>>> +		 * the SET_CUR one. IOW, what we expect is just a GET_CUR value
>>>>> +		 * differing from the saved one.
>>>>> +		 *
>>>>> +		 * Mixers of most devices are synchronous. The should have
>>>>> +		 * returned early without extra sleep. Asynchronous mixers will
>>>>> +		 * return once the accumulated time is enough for them to change
>>>>> +		 * the value.
>>>>> +		 */
>>>>> +		msleep(10);
>>>>>     	}
>>>>>     
>>>>> +	/* Check again after the last msleep(). */
>>>>> +	if (get_cur_mix_raw(cval, channel, &check))
>>>>> +		goto get_cur_broken;
>>>>> +	if (check != saved)
>>>>> +		return 0;
>>>>> +
>>>>>     	if (cval->head.mixer->chip->quirk_flags & QUIRK_FLAG_MIXER_GET_CUR_BROKEN) {
>>>>> +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),
>>>>>
>>>>> ---
>>>>> base-commit: 3eb40771c00a8488fa6ed2cc1fe203477908bf38
>>>>> change-id: 74676fce-uac-precise-sticky-check-94474a22b57d
>>>>>
>>>>> Thanks,
>>>>> Rong
>>>>>

  reply	other threads:[~2026-08-18 14:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-15 21:47 [PATCH] ALSA: usb-audio: Check sticky mixers precisely Rong Zhang
2026-08-16  5:14 ` Alexander Niemeyer
2026-08-16 13:50   ` Rong Zhang
     [not found]     ` <74ca2e17-8fb8-4ede-8e7e-441be815b5b6@gmx.de>
2026-08-16 15:08       ` Rong Zhang
2026-08-18 14:41         ` Alexander Niemeyer [this message]
2026-08-18 15:24           ` Alexander Niemeyer
2026-08-19 16:39             ` Rong Zhang
2026-08-19 18:37               ` Alexander Niemeyer
2026-08-20 12:51                 ` Rong Zhang
2026-08-20 15:01                   ` Alexander Niemeyer
2026-08-20 16:29                     ` Rong Zhang

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=0b04f4ea-03bf-4fad-a467-2b378331137f@gmx.de \
    --to=adventurefan@gmx.de \
    --cc=i@rong.moe \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    /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