Bug 211941
| Summary: | USB audio stuck hardware params | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Michael Forney (mforney) |
| Component: | Sound(ALSA) | Assignee: | Jaroslav Kysela (perex) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | tiwai |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 5.11 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Attachments: |
USB descriptors
5.10 dmesg output 5.11 dmesg output Test hw_params program /proc/asound/S3/stream0 Test fix patch Revised test patch |
||
Created attachment 295455 [details]
5.10 dmesg output
Created attachment 295457 [details]
5.11 dmesg output
Created attachment 295459 [details]
Test hw_params program
Assuming that your device is running in the implicit feedback mode (that's what commit affects), it's the right behavior. In the implicit fb mode, both full-duplex streams (or the streams sharing the same endpoint) have to be aligned with the same parameters, and that's the side-effect. See the LKML thread: https://lore.kernel.org/lkml/YDfYAYCaC9KDc1F0@fritha.org/ OK, now looking at the test program, I understood about the situation. I'd say that it's an application bug. If the application calls snd_pcm_hw_free() before re-setting the hw_params, it should work with the latest driver. The multiple hw_params calls work in most cases, but it essentially means to apply on top of the previous setup, hence such a side-effect can happen. If you redo the whole set up, you'd need to clear the setup at first. Thanks for taking a look. > Assuming that your device is running in the implicit feedback mode (that's > what commit affects), it's the right behavior. In the implicit fb mode, both > full-duplex streams (or the streams sharing the same endpoint) have to be > aligned with the same parameters, and that's the side-effect. I don't think this is the case. The device has only one output endpoint (half-duplex) with *explicit* feedback. It is not in implicit feedback mode. I will attach the /proc/asound/S3/stream0 file. I believe the reason why this commit introduces the behavior change is that get_sync_ep_from_substream can return an endpoint if cur_rate is non-zero, even if it is not implicit_fb. Previously, the code to detect the sync endpoint would run only once during setup_hw_info, but now it is run every time the hw_rule_*_implicit_fb functions are called. On the second configuration, the endpoint already has a cur_rate, so the implicit_fb rules begin to run, even though it is not implicit_fb (I hope I'm explaining well enough). > I'd say that it's an application bug. If the application calls > snd_pcm_hw_free() before re-setting the hw_params, it should work with the > latest driver. Thanks for the tip, adding snd_pcm_hw_free does seem to solve the problem. I will report this to the sndio developers. Created attachment 295473 [details]
/proc/asound/S3/stream0
Yes, that's another case where the cur_rate is evaluated, indeed. But essentially that's the same issue -- the endpoint is being used already by else (or itself without releasing), hence it cannot change the parameter since the endpoint setup is exclusive. In that sense, the driver behavior is correct. I see, thanks for the explanation. In that case, it seems like the hw_rule_*_implicit_fb functions are misnamed, since they apply to all endpoints, not just implicit feedback endpoints. Also, I think the alsa-lib documentation could be improved to prevent this type of issue in the future. snd_pcm_hw_params says that the configuration can't be changed once the stream is running, which implies that it *can* be changed before that. However, it does not mention that snd_pcm_hw_free is needed in between. I have sent a patch to the sndio mailing list: https://sndio.org/arch/0134.html Thinking of this problem again, this could be also worked around in the kernel side, too, although the behavior itself is basically an application bug. Below is the test patch. Could you check it without fixing the application side? Created attachment 295513 [details]
Test fix patch
Created attachment 295515 [details]
Revised test patch
> Could you check it without fixing the application side?
Yep, it fixes the issue with unpatched sndio.
OK, now I submitted and merge the fix for the upstream.
The commit 5f5e6a3e8b1df52f79122e447855cffbf1710540 in sound git tree.
ALSA: usb-audio: Allow modifying parameters with succeeding hw_params calls
|
Created attachment 295453 [details] USB descriptors After upgrading to linux 5.11, I noticed some strange behavior with setting hw_params on my USB audio device (Schiit Modi 3 DAC). If the parameters are set to some rate or period size, I can't change them later on to new values. I attached a test program to demonstrate the issue. On linux 5.10, the second hw_params can be set successfully: rate=48000 dir=0 period=4800 dir=0 rate=44100 dir=0 period=441 dir=0 but on linux 5.11, it prints: rate=48000 dir=0 period=4800 dir=0 rate=48000 dir=0 period=4800 dir=0 It seems the rate and period are getting stuck at the previous settings. I see there was quite a bit of refactoring of USB audio in 5.11. I bisected the issue to commit e4ea77f8e5 (ALSA: usb-audio: Always apply the hw constraints for implicit fb sync). This seems suspicious since the device does not have any implicit feedback endpoints; it has one asynchronous endpoint with explicit feedback. I attached a dump of the device descriptors. I also attached some snippets from dmesg with debug messages for 5.10 and 5.11. It shows that on 5.11, the second snd_pcm_hw_params triggers hw_rule_*_implicit_fb, which locks down the parameters to the previous values. I believe this is unexpected. I noticed this bug when using sndio, since it sets an initially large period size when the device is opened, before the application chooses its desired size, but it was getting stuck at 4800. This was causing firefox to play videos with reduced framerate.