Re: Regression playing 24bit/96kHz audio on USB audio interface between 5.10.94 and 5.15.21

[Takashi Iwai <tiwai@suse.de>]

On Sat, 31 Dec 2022 14:20:56 +0100,
Takashi Iwai wrote:
> 
> On Sat, 31 Dec 2022 13:10:18 +0100,
> Jaroslav Kysela wrote:
> > 
> > On 31. 12. 22 12:38, Ruud van Asseldonk wrote:
> > > Hi all,
> > > 
> > > I bisected this and identified e4ea77f8e53f9accb9371fba34c189d0447ecce0
> > > (ALSA: usb-audio: Always apply the hw constraints for implicit fb sync)
> > > as the first commit where it is no longer possible to change the sample
> > > rate. On the parent commit, my sample program successfully changes the
> > > sample rate from 44100 Hz to 96000 Hz, but on e4ea77f8e53f9, the second
> > > call to snd_pcm_hw_params fails.
> > 
> > There is an easy workaround - call snd_pcm_hw_free() between the
> > params setup. This call ensure that the sync ep is freed. I can just
> > confirm that this
> > problem is only for USB hw with the implicit feedback (tested with 6.0.11 kernel).
> > 
> > I will try to debug this when I find a little free time. Perhaps,
> > Takashi may have a straight idea, what's wrong. When only one stream
> > is used, this hw_params call should succeed, too.
> 
> The patch like below might help (note: totally untested).
> 
> My wild guess is that it's an internal stream start for the implicit
> fb case at prepare without actually starting a PCM stream, and it
> wasn't stopped properly before the second hw_params call.

Looking at the problem again, I think this is a different problem from
the patch I've sent.  This is rather the hw_params that becomes too
restrictive once after the implicit sync is set up.  A workaround
patch is below.

The previous patch is still valid, though.  I'm going to submit a
series of fixes.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: usb-audio: Relax hw constraints for implicit fb sync

The fix commit the commit e4ea77f8e53f ("ALSA: usb-audio: Always apply
the hw constraints for implicit fb sync") tried to address the bug
where an incorrect PCM parameter is chosen when two (implicit fb)
streams are set up at the same time.  This change had, however, some
side effect: once when the sync endpoint is chosen and set up, this
restriction is applied at the next hw params unless it's freed via hw
free explicitly.

This patch is a workaround for the problem by relaxing the hw
constraints a bit for the implicit fb sync.  We still keep applying
the hw constraints for implicit fb sync, but only when the matching
sync EP is being used by other streams.

Fixes: e4ea77f8e53f ("ALSA: usb-audio: Always apply the hw constraints for implicit fb sync")
Link: https://lore.kernel.org/r/4e509aea-e563-e592-e652-ba44af6733fe@veniogames.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/pcm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 7fc95ae9b2f0..2fd4ecc1b25a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -937,8 +937,13 @@ get_sync_ep_from_substream(struct snd_usb_substream *subs)
 			continue;
 		/* for the implicit fb, check the sync ep as well */
 		ep = snd_usb_get_endpoint(chip, fp->sync_ep);
-		if (ep && ep->cur_audiofmt)
-			return ep;
+		if (ep && ep->cur_audiofmt) {
+			/* ditto, if the sync (data) ep is used by others,
+			 * this stream is restricted by the sync ep
+			 */
+			if (ep != subs->sync_endpoint || ep->opened > 1)
+				return ep;
+		}
 	}
 	return NULL;
 }
-- 
2.35.3