Re: [PATCH] ALSA: usb-audio: Check sticky mixers precisely
Alexander Niemeyer <[email protected]>
| Newsgroups | gmane.linux.sound,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Rong, thanks. I have now completed the three tests you asked for. 1. Your sticky-check patch with the detachable Mic boom attached I tested the same minimal 7.1.8 backport of your ~16-value / 10 ms sticky-check logic again, this time with the detachable microphone boom physically attached. The external snd-usb-audio module was rebuilt and loaded successfully (the module taint was "O"), and the old 100 ms diagnostic changes were removed before this test. The result is still a failure: sticky mixer values (-16384/0/256 => -3840), disabling The PCM control exposes only: Capabilities: pswitch pswitch-joined and no Playback Volume control. I repeated this once more with another dongle replug, without changing the module or touching the volume wheel, and got the same result again. So: your sticky-check patch + Mic boom attached -> still classified as sticky -> reproduced twice 2. Is Playback SET_CUR really broken after Mic SET_RES with the boom detached? For this test I detached the microphone boom. I used a test version of snd-usb-audio which: - performed the Mic UAC1 resolution probe, - kept the Playback Volume control exposed, - clamped the known-broken advertised -64 dB minimum to -63 dB, - and bypassed the normal ALSA mixer cache so that mixer reads fetch GET_CUR from the device. The initial Playback value was: 43 = -20 dB I played a 3-second 1 kHz test tone and listened to its level. Then I requested: 23 = -40 dB The immediate output from the cset operation still showed: values=43 I waited two seconds and issued a fresh cget. It then returned: values=23 I played the exact same test tone again, and the second tone was clearly quieter. So Playback SET_CUR is physically effective. At least in this test, the effect after Mic SET_RES with the boom detached is not that Playback SET_CUR stops working. Instead, GET_CUR can remain stale for some time after SET_CUR and later reflect the actual value. In short: before: 43 (-20 dB) SET_CUR requested: 23 (-40 dB) immediate result: 43 GET_CUR after 2 s: 23 physical volume: clearly quieter 3. QUIRK_FLAG_MIXER_GET_CUR_BROKEN and mixer change notifications Linux 7.1.8 does not contain QUIRK_FLAG_MIXER_GET_CUR_BROKEN yet, so for this test I made a minimal backport of the flag behavior to the otherwise stock 7.1.8 mixer code, without your new sticky-check patch. I loaded snd-usb-audio with: quirk_flags=046d:0aba:mixer_get_cur_broken The kernel detected the constant GET_CUR condition and kept the Playback Volume control exposed: sticky mixer values (-16384/0/256 => -3840), disabling PROXTESTC: treating constant GET_CUR as broken For this test the microphone boom was attached. Before turning the physical headset wheel: ALSA cget: 56 wpctl: Volume 0.72 I then deliberately turned the physical volume wheel further down. Afterwards: ALSA cget: 47 wpctl: Volume 0.52 KDE's volume OSD also reacted normally while I turned the wheel. So, on this device, using MIXER_GET_CUR_BROKEN did not break propagation of physical mixer changes through the audio stack: physical wheel -> ALSA value changed -> PipeWire value changed -> KDE OSD reacted normally Therefore the current picture seems to be: - Your revised sticky check still rejects the Playback Volume control, both with the detachable Mic boom attached and detached. - With the Mic boom detached, Mic SET_RES can make Playback GET_CUR stay stale after a Playback SET_CUR, but Playback SET_CUR itself is still physically effective. - At least on this Logitech device, treating GET_CUR as broken did not prevent physical wheel changes from reaching ALSA/PipeWire/KDE. Let me know what you would like me to test next. I have kept the Fedora test environment and module-signing setup intact. Thanks, Alexander Am 19.08.2026 um 18:39 schrieb Rong Zhang: > Hi Alexander, > > On Tue, 2026-08-18 at 17:24 +0200, Alexander Niemeyer wrote: >> Hi Rong, >> >> one important follow-up to my previous message: >> >> I realized that the detachable microphone boom was not physically connected >> to the headset during the direct libusb tests I reported earlier. >> >> I therefore repeated the relevant tests with the boom microphone attached, >> and the result changes in an important way. >> >> With the boom microphone attached, on a fresh device: >> >> Mic GET_RES = 256 >> no SET_RES >> >> Playback: >> GET_CUR = -3840 (-15 dB) >> SET_CUR = -2048 (-8 dB) >> GET_CUR changes to -2048 after 65.3 ms >> >> Result: PASS >> >> After another power cycle, with the boom still attached: >> >> 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 changes to -2048 after 66.3 ms >> >> Result: PASS >> >> I also repeated the complete Mic SET_RES sequence: >> >> 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, GET_RES remains 256, and Playback >> SET_CUR(-8 dB) >> still works afterwards, with GET_CUR changing after 76.0 ms. >> >> However, with the boom microphone attached, the stock Fedora 7.1.8 >> snd-usb-audio driver still reproduces the original failure: >> >> sticky mixer values (-16384/0/256 => -3840), disabling >> >> and no PCM Playback Volume control is exposed. > Could you test my patch with Mic attached? > >> So I need to correct the conclusion from my previous message: >> >> The Mic SET_RES behavior is real, but it depends on whether the detachable >> microphone boom is physically connected. >> >> With the boom disconnected: >> a single Mic SET_RES(128) is sufficient to make subsequent Playback >> SET_CUR ineffective for more than 1200 ms. >> >> With the boom connected: >> both a single Mic SET_RES(128) and the full SET_RES sequence leave >> Playback SET_CUR working normally. >> > Thanks for your tests. > > Let me conclude: > > - Mic detached: SET_RES on Mic breaks Playback > - Mic attached: SET_RES on Mic breaks nothing > > Still, I doubt if Playback SET_CUR is really broken in the first case. > There is a chance that Playback SET_CUR is effective while GET_CUR > becomes broken. > > Could you test: > > - Leave the Mic detached > - SET_RES on Mic > - GET_CUR on Playback > - Play some audio, listen to it > - SET_CUR on Playback > - GET_CUR on Playback > - Play some audio again, listen to it, and tell if the physical volume > has changed > > The procedure can be achieved by hacking into the snd-usb-audio driver > and nullifying its cache mechanism, so that you can fetch GET_CUR values > from standard ALSA mixer interface. > > If you can notice physical volume changes, it implies SET_RES on Mic > break Playback GET_CUR without breaking SET_CUR. Otherwise, it break > Playback SET_CUR. > > If it only breaks GET_CUR, it implies the methodology of sticky check is > problematic. In this case I would probably consider demoting the sticky > check's severity. > > I am also thinking about how we should demote the severity. Could you > also test if applying QUIRK_FLAG_MIXER_GET_CUR_BROKEN (without this > patch) breaks mixer change notifications? I.e., tune the volume with the > buttons/knobs on the device and see if the audio stack reflects the > change. I guess it will break the notifications, but I would still like > to wait for your confirmation. > > Thanks, > Rong > >> Therefore Mic SET_RES alone does not explain the complete snd-usb-audio >> failure in all device configurations. There must be another difference in >> the full snd-usb-audio initialization path when the boom is attached. >> >> The separate Playback -64 dB minimum issue I reported remains unchanged: >> -64 dB is ineffective, -63 dB works, and attempting -64 dB does not prevent >> subsequent valid Playback SET_CUR requests from working. >> >> Sorry for not noticing the detachable-microphone state earlier. I wanted to >> send this correction as soon as I confirmed it. >> >> I still have the usbmon/pcapng captures and can run further targeted tests >> if useful. >> >> Thanks, >> Alexander >> >> Am 18.08.2026 um 16:41 schrieb Alexander Niemeyer: >>> 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<[email protected]> >>>>>>>> Closes:https://lore.kernel.org/r/[email protected] >>>>>>>> >>>>>>>> Signed-off-by: Rong Zhang<[email protected]> >>>>>>>> --- >>>>>>>> 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 >>>>>>>>