missing sound on kernel-5.15
I am testing many headsets on Android platform with different kernel versions.
On kernel-5.10, calling sequence for playback is 1. snd_usb_hw_params -> set highest sampling rate, no SAMPLING_FREQ_CONTROL USB request sent in this stage 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, still no SAMPLING_FREQ_CONTROL USB request 4. snd_usb_pcm_parepare -> in configure_endpoint SAMPLING_FREQ_CONTROL USB request was sent on USB bus
With the same calling sequence, a slightly behavior change on kernel- 5.15 is 1. snd_usb_hw_params -> set highest sampling rate, SAMPLING_FREQ_CONTROL USB request was sent on USB bus 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, SAMPLING_FREQ_CONTROL USB request was sent too (because configure_endpoint was moved to snd_usb_hw_params function) 4. snd_usb_pcm_parepare -> no SAMPLING_FREQ_CONTROL USB request because of USB EP flag check
I checked all USB packets and confirmed audio data output was correct. But there may be missing sound problem for the first sound with many headsets (for example Samsung headset). I found this issue is related to two-times sampling rate set request. (I tried to forcibly skip first USB request, everything became okay.) So do you guys know why configure_endpoint was moved to snd_usb_hw_params? Or could you provide patch for this problem?
Many thanks Chihhao
On Mon, Aug 22, 2022 at 04:06:58PM +0800, chihhao chen wrote:
I am testing many headsets on Android platform with different kernel versions.
On kernel-5.10, calling sequence for playback is
- snd_usb_hw_params -> set highest sampling rate, no
SAMPLING_FREQ_CONTROL USB request sent in this stage 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, still no SAMPLING_FREQ_CONTROL USB request 4. snd_usb_pcm_parepare -> in configure_endpoint SAMPLING_FREQ_CONTROL USB request was sent on USB bus
With the same calling sequence, a slightly behavior change on kernel- 5.15 is
- snd_usb_hw_params -> set highest sampling rate,
SAMPLING_FREQ_CONTROL USB request was sent on USB bus 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, SAMPLING_FREQ_CONTROL USB request was sent too (because configure_endpoint was moved to snd_usb_hw_params function) 4. snd_usb_pcm_parepare -> no SAMPLING_FREQ_CONTROL USB request because of USB EP flag check
I checked all USB packets and confirmed audio data output was correct. But there may be missing sound problem for the first sound with many headsets (for example Samsung headset). I found this issue is related to two-times sampling rate set request. (I tried to forcibly skip first USB request, everything became okay.) So do you guys know why configure_endpoint was moved to snd_usb_hw_params?
Hi Chihhao,
the commit you're looking for is bf6313a0ff766925462e97b4e733d5952de02367 ("ALSA: usb-audio: Refactor endpoint management")
Good luck, Geraldo Nascimento
Or could you provide patch for this problem?
Many thanks Chihhao
Hi Takashi,
Because of this problem, is it okay to remove configure_endpoints function call in snd_usb_hw_params?
Then configure_endpoints would be called in snd_usb_pcm_prepare and USB control transfers become the same as which in kernel-5.10. There will be only one SAMPLING_FREQ_CONTROL request and no sound-missing phenomenon.
Thanks
On Mon, 2022-08-22 at 08:57 -0300, Geraldo Nascimento wrote:
On Mon, Aug 22, 2022 at 04:06:58PM +0800, chihhao chen wrote:
I am testing many headsets on Android platform with different kernel versions.
On kernel-5.10, calling sequence for playback is
- snd_usb_hw_params -> set highest sampling rate, no
SAMPLING_FREQ_CONTROL USB request sent in this stage 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, still no SAMPLING_FREQ_CONTROL USB request 4. snd_usb_pcm_parepare -> in configure_endpoint SAMPLING_FREQ_CONTROL USB request was sent on USB bus
With the same calling sequence, a slightly behavior change on kernel- 5.15 is
- snd_usb_hw_params -> set highest sampling rate,
SAMPLING_FREQ_CONTROL USB request was sent on USB bus 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, SAMPLING_FREQ_CONTROL USB request was sent too (because configure_endpoint was moved to snd_usb_hw_params function) 4. snd_usb_pcm_parepare -> no SAMPLING_FREQ_CONTROL USB request because of USB EP flag check
I checked all USB packets and confirmed audio data output was correct. But there may be missing sound problem for the first sound with many headsets (for example Samsung headset). I found this issue is related to two-times sampling rate set request. (I tried to forcibly skip first USB request, everything became okay.) So do you guys know why configure_endpoint was moved to snd_usb_hw_params?
Hi Chihhao,
the commit you're looking for is bf6313a0ff766925462e97b4e733d5952de02367 ("ALSA: usb-audio: Refactor endpoint management")
Good luck, Geraldo Nascimento
Or could you provide patch for this problem?
Many thanks Chihhao
On Mon, 29 Aug 2022 09:56:42 +0200, chihhao chen wrote:
Hi Takashi,
Because of this problem, is it okay to remove configure_endpoints function call in snd_usb_hw_params?
Then configure_endpoints would be called in snd_usb_pcm_prepare and USB control transfers become the same as which in kernel-5.10. There will be only one SAMPLING_FREQ_CONTROL request and no sound-missing phenomenon.
Are you sure that removing the configure_endpoints() fixes the problem?
Takashi
Thanks
On Mon, 2022-08-22 at 08:57 -0300, Geraldo Nascimento wrote:
On Mon, Aug 22, 2022 at 04:06:58PM +0800, chihhao chen wrote:
I am testing many headsets on Android platform with different kernel versions.
On kernel-5.10, calling sequence for playback is
- snd_usb_hw_params -> set highest sampling rate, no
SAMPLING_FREQ_CONTROL USB request sent in this stage 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, still no SAMPLING_FREQ_CONTROL USB request 4. snd_usb_pcm_parepare -> in configure_endpoint SAMPLING_FREQ_CONTROL USB request was sent on USB bus
With the same calling sequence, a slightly behavior change on kernel- 5.15 is
- snd_usb_hw_params -> set highest sampling rate,
SAMPLING_FREQ_CONTROL USB request was sent on USB bus 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, SAMPLING_FREQ_CONTROL USB request was sent too (because configure_endpoint was moved to snd_usb_hw_params function) 4. snd_usb_pcm_parepare -> no SAMPLING_FREQ_CONTROL USB request because of USB EP flag check
I checked all USB packets and confirmed audio data output was correct. But there may be missing sound problem for the first sound with many headsets (for example Samsung headset). I found this issue is related to two-times sampling rate set request. (I tried to forcibly skip first USB request, everything became okay.) So do you guys know why configure_endpoint was moved to snd_usb_hw_params?
Hi Chihhao,
the commit you're looking for is bf6313a0ff766925462e97b4e733d5952de02367 ("ALSA: usb-audio: Refactor endpoint management")
Good luck, Geraldo Nascimento
Or could you provide patch for this problem?
Many thanks Chihhao
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
K5.15 new behavior snd_usb_hw_params -> configure_endpoints() -> issue SAMPLING_FREQ_CONTROL USB request
Thanks
On Mon, 2022-08-29 at 10:06 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 09:56:42 +0200, chihhao chen wrote:
Hi Takashi,
Because of this problem, is it okay to remove configure_endpoints function call in snd_usb_hw_params?
Then configure_endpoints would be called in snd_usb_pcm_prepare and USB control transfers become the same as which in kernel-5.10. There will be only one SAMPLING_FREQ_CONTROL request and no sound-missing phenomenon.
Are you sure that removing the configure_endpoints() fixes the problem?
Takashi
Thanks
On Mon, 2022-08-22 at 08:57 -0300, Geraldo Nascimento wrote:
On Mon, Aug 22, 2022 at 04:06:58PM +0800, chihhao chen wrote:
I am testing many headsets on Android platform with different kernel versions.
On kernel-5.10, calling sequence for playback is
- snd_usb_hw_params -> set highest sampling rate, no
SAMPLING_FREQ_CONTROL USB request sent in this stage 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, still no SAMPLING_FREQ_CONTROL USB request 4. snd_usb_pcm_parepare -> in configure_endpoint SAMPLING_FREQ_CONTROL USB request was sent on USB bus
With the same calling sequence, a slightly behavior change on kernel- 5.15 is
- snd_usb_hw_params -> set highest sampling rate,
SAMPLING_FREQ_CONTROL USB request was sent on USB bus 2. snd_pcm_release 3. snd_usb_hw_params -> set proper sampling rate, SAMPLING_FREQ_CONTROL USB request was sent too (because configure_endpoint was moved to snd_usb_hw_params function) 4. snd_usb_pcm_parepare -> no SAMPLING_FREQ_CONTROL USB request because of USB EP flag check
I checked all USB packets and confirmed audio data output was correct. But there may be missing sound problem for the first sound with many headsets (for example Samsung headset). I found this issue is related to two-times sampling rate set request. (I tried to forcibly skip first USB request, everything became okay.) So do you guys know why configure_endpoint was moved to snd_usb_hw_params?
Hi Chihhao,
the commit you're looking for is bf6313a0ff766925462e97b4e733d5952de02367 ("ALSA: usb-audio: Refactor endpoint management")
Good luck, Geraldo Nascimento
Or could you provide patch for this problem?
Many thanks Chihhao
On Mon, 29 Aug 2022 10:50:58 +0200, chihhao chen wrote:
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
Hm, is it a UAC1 device? Such a device should work with multiple SAMPLING_FREQ_CONTROL invocations, but some device might be not tolerant or buggy... The multiple init_sample_rate() invocations may happen with the older kernel under certain situations, so maybe we need a different fix.
How about the patch like below?
thanks,
Takashi
--- diff --git a/sound/usb/card.h b/sound/usb/card.h index ca75f2206170..507cd62f0ff8 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -134,6 +134,7 @@ struct snd_usb_endpoint { /* for hw constraints */ const struct audioformat *cur_audiofmt; unsigned int cur_rate; + unsigned int cur_rate_setup; snd_pcm_format_t cur_format; unsigned int cur_channels; unsigned int cur_frame_bytes; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0d7b73bf7945..1a5a9bc98a96 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -806,6 +806,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, ep->cur_audiofmt = fp; ep->cur_channels = fp->channels; ep->cur_rate = params_rate(params); + ep->cur_rate_setup = 0; ep->cur_format = params_format(params); ep->cur_frame_bytes = snd_pcm_format_physical_width(ep->cur_format) * ep->cur_channels / 8; @@ -928,6 +929,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, ep->altsetting = 0; ep->cur_audiofmt = NULL; ep->cur_rate = 0; + ep->cur_rate_setup = 0; ep->iface_ref = NULL; ep->clock_ref = NULL; usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num); @@ -1356,6 +1358,9 @@ static int init_sample_rate(struct snd_usb_audio *chip, struct snd_usb_clock_ref *clock = ep->clock_ref; int err;
+ if (ep->cur_rate == ep->cur_rate_setup) + return 0; + if (clock) { if (atomic_read(&clock->locked)) return 0; @@ -1374,6 +1379,7 @@ static int init_sample_rate(struct snd_usb_audio *chip,
if (clock) clock->rate = ep->cur_rate; + ep->cur_rate_setup = ep->cur_rate; return 0; }
On Mon, 29 Aug 2022 14:16:27 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 10:50:58 +0200, chihhao chen wrote:
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
Hm, is it a UAC1 device? Such a device should work with multiple SAMPLING_FREQ_CONTROL invocations, but some device might be not tolerant or buggy... The multiple init_sample_rate() invocations may happen with the older kernel under certain situations, so maybe we need a different fix.
How about the patch like below?
It's missing the clearance for suspend/resume. The revised patch is below.
Takashi
-- 8< -- diff --git a/sound/usb/card.h b/sound/usb/card.h index ca75f2206170..507cd62f0ff8 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -134,6 +134,7 @@ struct snd_usb_endpoint { /* for hw constraints */ const struct audioformat *cur_audiofmt; unsigned int cur_rate; + unsigned int cur_rate_setup; snd_pcm_format_t cur_format; unsigned int cur_channels; unsigned int cur_frame_bytes; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0d7b73bf7945..58ca1f920972 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -806,6 +806,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, ep->cur_audiofmt = fp; ep->cur_channels = fp->channels; ep->cur_rate = params_rate(params); + ep->cur_rate_setup = 0; ep->cur_format = params_format(params); ep->cur_frame_bytes = snd_pcm_format_physical_width(ep->cur_format) * ep->cur_channels / 8; @@ -928,6 +929,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, ep->altsetting = 0; ep->cur_audiofmt = NULL; ep->cur_rate = 0; + ep->cur_rate_setup = 0; ep->iface_ref = NULL; ep->clock_ref = NULL; usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num); @@ -943,6 +945,7 @@ void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep) ep->iface_ref->need_setup = true; if (ep->clock_ref) ep->clock_ref->rate = 0; + ep->cur_rate_setup = 0; }
/* @@ -1356,6 +1359,9 @@ static int init_sample_rate(struct snd_usb_audio *chip, struct snd_usb_clock_ref *clock = ep->clock_ref; int err;
+ if (ep->cur_rate == ep->cur_rate_setup) + return 0; + if (clock) { if (atomic_read(&clock->locked)) return 0; @@ -1374,6 +1380,7 @@ static int init_sample_rate(struct snd_usb_audio *chip,
if (clock) clock->rate = ep->cur_rate; + ep->cur_rate_setup = ep->cur_rate; return 0; }
On Mon, 29 Aug 2022 20:15:33 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 14:16:27 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 10:50:58 +0200, chihhao chen wrote:
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
Hm, is it a UAC1 device? Such a device should work with multiple SAMPLING_FREQ_CONTROL invocations, but some device might be not tolerant or buggy... The multiple init_sample_rate() invocations may happen with the older kernel under certain situations, so maybe we need a different fix.
How about the patch like below?
It's missing the clearance for suspend/resume. The revised patch is below.
... and after reading the mail again, I noticed that it's all rubbish, scratch the previous ones.
Have you tested it with the later kernel? I guess this has been already addressed. In the recent kernel, the rate is set per assigned clock, hence it won't be set up twice unnecessarily.
Takashi
On Tue, 30 Aug 2022 07:54:59 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 20:15:33 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 14:16:27 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 10:50:58 +0200, chihhao chen wrote:
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
Hm, is it a UAC1 device? Such a device should work with multiple SAMPLING_FREQ_CONTROL invocations, but some device might be not tolerant or buggy... The multiple init_sample_rate() invocations may happen with the older kernel under certain situations, so maybe we need a different fix.
How about the patch like below?
It's missing the clearance for suspend/resume. The revised patch is below.
... and after reading the mail again, I noticed that it's all rubbish, scratch the previous ones.
Have you tested it with the later kernel? I guess this has been already addressed. In the recent kernel, the rate is set per assigned clock, hence it won't be set up twice unnecessarily.
That said, try to cherry-pick the upstream commit c11117b634f4f832c4420d3cf41c44227f140ce1.
If this is confirmed to work, we can ask Greg to merge it into 5.15.y stable tree.
Takashi
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Which kernel version should I try? kernel-5.19 or?
Thanks
On Tue, 2022-08-30 at 07:54 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 20:15:33 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 14:16:27 +0200, Takashi Iwai wrote:
On Mon, 29 Aug 2022 10:50:58 +0200, chihhao chen wrote:
Hi Takashi,
Yes.
To issue SAMPLING_FREQ_CONTROL USB request two times is root cause of this issue.
Hm, is it a UAC1 device? Such a device should work with multiple SAMPLING_FREQ_CONTROL invocations, but some device might be not tolerant or buggy... The multiple init_sample_rate() invocations may happen with the older kernel under certain situations, so maybe we need a different fix.
How about the patch like below?
It's missing the clearance for suspend/resume. The revised patch is below.
... and after reading the mail again, I noticed that it's all rubbish, scratch the previous ones.
Have you tested it with the later kernel? I guess this has been already addressed. In the recent kernel, the rate is set per assigned clock, hence it won't be set up twice unnecessarily.
Takashi
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF
Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF
Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel-5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
Hi Takashi,
I will try to insall ALSA tools.
I am testing these typec headsets on ARM8 smart phone platform. Because most x86 laptops support only 3.5mm input, I cannot test these typec devices. This problem should have something to do with host. If I remove configure_endpoints() in snd_usb_hw_params() like k5.10 behavior, this problem does not happen. It seems that to set frequency multiple times leads to headset abnormal phenomenon.
Thanks
On Wed, 2022-08-31 at 07:18 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote:
Hi Takashi,
I tried the patch but this problem still happens.
I add some logs in snd_usb_init_sample_rate() in kernel- 5.10 [ 146.260105][T1702328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 96000, clock 0 protocol 0 [ 146.289892][T1002328] writer: usb 1-1: [name:snd_usb_audio&]2:2 Set sample rate 48000, clock 0 protocol 0
Because TinyAlsa tends to set highest rate for initialization and real rate for playback, it will still trigger two-times SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
On Wed, 31 Aug 2022 09:03:03 +0200, chihhao chen wrote:
Hi Takashi,
I will try to insall ALSA tools.
I am testing these typec headsets on ARM8 smart phone platform. Because most x86 laptops support only 3.5mm input, I cannot test these typec devices.
Hm, but it's a USB-audio device, can't it just be plugged to a laptop...?
This problem should have something to do with host. If I remove configure_endpoints() in snd_usb_hw_params() like k5.10 behavior, this problem does not happen. It seems that to set frequency multiple times leads to headset abnormal phenomenon.
I understand that part, but it's still weird. IIUC, it happens after closing the previous stream and start a new stream, right? Then can you play two different rates on 5.10 kernel; e.g. at first call snd_pcm_prepare with a high rate and stop/close immediately. Then start quickly the stream in a normal rate again. That should issue the sample rate changes in a similar way, and this should cause the same problem, if it's really about the call of endpoint / rate setups.
Or if it's more or less a timing issue, you might try to apply the delay quirk such as QUIRK_FLAG_CTL_MSG_DELAY_XXX or QUIRK_FLAG_IFACE_DELAY.
Takashi
Thanks
On Wed, 2022-08-31 at 07:18 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 08:13:44 +0200, chihhao chen wrote: > > Hi Takashi, > > I tried the patch but this problem still happens. > > I add some logs in snd_usb_init_sample_rate() in kernel- > 5.10 > [ 146.260105][T1702328] writer: usb 1-1: > [name:snd_usb_audio&]2:2 > Set > sample rate 96000, clock 0 protocol 0 > [ 146.289892][T1002328] writer: usb 1-1: > [name:snd_usb_audio&]2:2 > Set > sample rate 48000, clock 0 protocol 0 > > Because TinyAlsa tends to set highest rate for > initialization > and > real > rate for playback, it will still trigger two-times > SAMPLING_FREQ_CONTROL USB requests.
Then this is a firmware problem of your device. The same problem would happen even with the old kernel if you run the application with different sample rates. Does the device work with 96kHz at all?
Could you give the lsusb -v output of the device, too?
> Which kernel version should I try? kernel-5.19 or?
Yes, 5.19 should suffice.
Takashi
Hi Takashi,
It happens as follows on Android platform 1. When users want to play a sound, Android system will call pcm_open to get highest sample rate. In this function it uses ioctl : SNDRV_PCM_IOCTL_HW_PARAMS to collect information and triggers set-rate behavior. In this stage, system just initializes many audio parameters but not start to play yet. 2. Android system calls pcm_close 3. Android system calls pcm_open with suitable sample rate parameter and triggers set-rate again.
(pcm_open and pcm_close are functions in tinyalsa on Android.)
Users cannot not start and close immediately because Android system always auto-close stream if there is no sound for 3 seconds.
Yes I think this may be a timing issue. But it takes to much delay time to solve this phenomenon. I tested to add delay time and found 1. pcm_open to get highest sample rate 2. delay 150ms 3. pcm_close 4. pcm_open again This delay time results in severe playback latency.
Thanks
On Wed, 2022-08-31 at 10:04 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 09:03:03 +0200, chihhao chen wrote:
Hi Takashi,
I will try to insall ALSA tools.
I am testing these typec headsets on ARM8 smart phone platform. Because most x86 laptops support only 3.5mm input, I cannot test these typec devices.
Hm, but it's a USB-audio device, can't it just be plugged to a laptop...?
This problem should have something to do with host. If I remove configure_endpoints() in snd_usb_hw_params() like k5.10 behavior, this problem does not happen. It seems that to set frequency multiple times leads to headset abnormal phenomenon.
I understand that part, but it's still weird. IIUC, it happens after closing the previous stream and start a new stream, right? Then can you play two different rates on 5.10 kernel; e.g. at first call snd_pcm_prepare with a high rate and stop/close immediately. Then start quickly the stream in a normal rate again. That should issue the sample rate changes in a similar way, and this should cause the same problem, if it's really about the call of endpoint / rate setups.
Or if it's more or less a timing issue, you might try to apply the delay quirk such as QUIRK_FLAG_CTL_MSG_DELAY_XXX or QUIRK_FLAG_IFACE_DELAY.
Takashi
Thanks
On Wed, 2022-08-31 at 07:18 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote:
Hi Takashi,
I also think it should be a firmware problem but it happens with many different devices because of new set sampling rate behavior in k5.15.
Device 1 UAC1 [ 134.924359][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device found, idVendor=04e8, idProduct=a04f, bcdDevice= 1.00 [ 134.925944][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 134.927338][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Product: Samsung USB C Earphone [ 134.928426][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]Manufacturer: bestechnic [ 134.929432][T1000005] kworker/0:0: usb 1-1: [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
Device 2 UAC3 [ 779.645324][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11 [ 779.647376][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 779.649492][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Product: USB-C to 3.5mm Headphone Jack Adapter [ 779.652262][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]Manufacturer: Apple, Inc. [ 779.652273][T1003414] kworker/0:1: usb 1-1: [name:usbcore&]SerialNumber: DWH126301CLJKLTAF Device 3 A XiaoMi adapter but it not in my hand now.
I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
Thanks
On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote: > On Tue, 30 Aug 2022 08:13:44 +0200, > chihhao chen wrote: > > > > Hi Takashi, > > > > I tried the patch but this problem still happens. > > > > I add some logs in snd_usb_init_sample_rate() in > > kernel- > > 5.10 > > [ 146.260105][T1702328] writer: usb 1-1: > > [name:snd_usb_audio&]2:2 > > Set > > sample rate 96000, clock 0 protocol 0 > > [ 146.289892][T1002328] writer: usb 1-1: > > [name:snd_usb_audio&]2:2 > > Set > > sample rate 48000, clock 0 protocol 0 > > > > Because TinyAlsa tends to set highest rate for > > initialization > > and > > real > > rate for playback, it will still trigger two-times > > SAMPLING_FREQ_CONTROL USB requests. > > Then this is a firmware problem of your device. > The same problem would happen even with the old kernel if > you > run > the > application with different sample rates. Does the device > work > with > 96kHz at all? > > Could you give the lsusb -v output of the device, too? > > > Which kernel version should I try? kernel-5.19 or? > > Yes, 5.19 should suffice. > > > Takashi
On Wed, 31 Aug 2022 11:26:44 +0200, chihhao chen wrote:
Hi Takashi,
It happens as follows on Android platform
- When users want to play a sound, Android system will call pcm_open
to get highest sample rate. In this function it uses ioctl : SNDRV_PCM_IOCTL_HW_PARAMS to collect information and triggers set-rate behavior. In this stage, system just initializes many audio parameters but not start to play yet. 2. Android system calls pcm_close 3. Android system calls pcm_open with suitable sample rate parameter and triggers set-rate again.
(pcm_open and pcm_close are functions in tinyalsa on Android.)
Users cannot not start and close immediately because Android system always auto-close stream if there is no sound for 3 seconds.
Yes I think this may be a timing issue. But it takes to much delay time to solve this phenomenon. I tested to add delay time and found
- pcm_open to get highest sample rate
- delay 150ms
- pcm_close
- pcm_open again
This delay time results in severe playback latency.
OK, and it's only about the missing sound, and no error is reported from the system itself, right? And at which code path did you put the delay of 150ms?
Also, as a blind shot, what if you use the coherency buffer by passing vmalloc=0 option to snd-usb-audio module?
Takashi
Thanks
On Wed, 2022-08-31 at 10:04 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 09:03:03 +0200, chihhao chen wrote:
Hi Takashi,
I will try to insall ALSA tools.
I am testing these typec headsets on ARM8 smart phone platform. Because most x86 laptops support only 3.5mm input, I cannot test these typec devices.
Hm, but it's a USB-audio device, can't it just be plugged to a laptop...?
This problem should have something to do with host. If I remove configure_endpoints() in snd_usb_hw_params() like k5.10 behavior, this problem does not happen. It seems that to set frequency multiple times leads to headset abnormal phenomenon.
I understand that part, but it's still weird. IIUC, it happens after closing the previous stream and start a new stream, right? Then can you play two different rates on 5.10 kernel; e.g. at first call snd_pcm_prepare with a high rate and stop/close immediately. Then start quickly the stream in a normal rate again. That should issue the sample rate changes in a similar way, and this should cause the same problem, if it's really about the call of endpoint / rate setups.
Or if it's more or less a timing issue, you might try to apply the delay quirk such as QUIRK_FLAG_CTL_MSG_DELAY_XXX or QUIRK_FLAG_IFACE_DELAY.
Takashi
Thanks
On Wed, 2022-08-31 at 07:18 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote:
On Tue, 30 Aug 2022 10:08:51 +0200, chihhao chen wrote: > > Hi Takashi, > > I also think it should be a firmware problem but it happens > with > many > different devices because of new set sampling rate behavior > in > k5.15. > > Device 1 UAC1 > [ 134.924359][T1000005] kworker/0:0: usb 1-1: > [name:usbcore&]New > USB > device found, idVendor=04e8, idProduct=a04f, bcdDevice= > 1.00 > [ 134.925944][T1000005] kworker/0:0: usb 1-1: > [name:usbcore&]New > USB > device strings: Mfr=1, Product=2, SerialNumber=3 > [ 134.927338][T1000005] kworker/0:0: usb 1-1: > [name:usbcore&]Product: > Samsung USB C Earphone > [ 134.928426][T1000005] kworker/0:0: usb 1-1: > [name:usbcore&]Manufacturer: bestechnic > [ 134.929432][T1000005] kworker/0:0: usb 1-1: > [name:usbcore&]SerialNumber: 20160406.1
Does this show the same problem? If so, that's interesting because UAC1 has a completely different way of setting the sample rate.
> Device 2 UAC3 > [ 779.645324][T1003414] kworker/0:1: usb 1-1: > [name:usbcore&]New > USB > device found, idVendor=05ac, idProduct=110a, > bcdDevice=26.11 > [ 779.647376][T1003414] kworker/0:1: usb 1-1: > [name:usbcore&]New > USB > device strings: Mfr=1, Product=2, SerialNumber=3 > [ 779.649492][T1003414] kworker/0:1: usb 1-1: > [name:usbcore&]Product: > USB-C to 3.5mm Headphone Jack Adapter > [ 779.652262][T1003414] kworker/0:1: usb 1-1: > [name:usbcore&]Manufacturer: Apple, Inc. > [ 779.652273][T1003414] kworker/0:1: usb 1-1: > [name:usbcore&]SerialNumber: DWH126301CLJKLTAF > Device 3 > A XiaoMi adapter but it not in my hand now. > > I will try to integrate k5.19 into my codebase.
At best, please give the alsa-info.sh output from each device. Run the script with --no-upload option and attach the output.
Then try to test whether the reported highest sample rate actually works as-is. That is, to see whether the problem is really about issuing the frequency change multiple times for different rates, or it's because issuing the highest rate screws up the device.
And, for UAC2/3 devices, it might be worth to try some known quirks, e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU (UAC2) devices. It's a bit 12 of quirk_flags option value.
Takashi
> > Thanks > > > On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote: > > On Tue, 30 Aug 2022 08:13:44 +0200, > > chihhao chen wrote: > > > > > > Hi Takashi, > > > > > > I tried the patch but this problem still happens. > > > > > > I add some logs in snd_usb_init_sample_rate() in > > > kernel- > > > 5.10 > > > [ 146.260105][T1702328] writer: usb 1-1: > > > [name:snd_usb_audio&]2:2 > > > Set > > > sample rate 96000, clock 0 protocol 0 > > > [ 146.289892][T1002328] writer: usb 1-1: > > > [name:snd_usb_audio&]2:2 > > > Set > > > sample rate 48000, clock 0 protocol 0 > > > > > > Because TinyAlsa tends to set highest rate for > > > initialization > > > and > > > real > > > rate for playback, it will still trigger two-times > > > SAMPLING_FREQ_CONTROL USB requests. > > > > Then this is a firmware problem of your device. > > The same problem would happen even with the old kernel if > > you > > run > > the > > application with different sample rates. Does the device > > work > > with > > 96kHz at all? > > > > Could you give the lsusb -v output of the device, too? > > > > > Which kernel version should I try? kernel-5.19 or? > > > > Yes, 5.19 should suffice. > > > > > > Takashi
Hi Takashi,
Yes no error reported and data on USB bus is also complete. (Use USB analyzer to collect packets on bus and check these data.)
I added delay right after find_substream_format() in snd_usb_hw_params() as follows 1. first time call snd_usb_hw_params(), do nothing 2. second time call snd_usb_hw_params(), delay 150ms after find_substream_format()
I tried to set snd_usb_use_vmalloc false but this problem still happened.
Thanks
On Wed, 2022-08-31 at 12:48 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 11:26:44 +0200, chihhao chen wrote:
Hi Takashi,
It happens as follows on Android platform
- When users want to play a sound, Android system will call
pcm_open to get highest sample rate. In this function it uses ioctl : SNDRV_PCM_IOCTL_HW_PARAMS to collect information and triggers set- rate behavior. In this stage, system just initializes many audio parameters but not start to play yet. 2. Android system calls pcm_close 3. Android system calls pcm_open with suitable sample rate parameter and triggers set-rate again.
(pcm_open and pcm_close are functions in tinyalsa on Android.)
Users cannot not start and close immediately because Android system always auto-close stream if there is no sound for 3 seconds.
Yes I think this may be a timing issue. But it takes to much delay time to solve this phenomenon. I tested to add delay time and found
- pcm_open to get highest sample rate
- delay 150ms
- pcm_close
- pcm_open again
This delay time results in severe playback latency.
OK, and it's only about the missing sound, and no error is reported from the system itself, right? And at which code path did you put the delay of 150ms?
Also, as a blind shot, what if you use the coherency buffer by passing vmalloc=0 option to snd-usb-audio module?
Takashi
Thanks
On Wed, 2022-08-31 at 10:04 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 09:03:03 +0200, chihhao chen wrote:
Hi Takashi,
I will try to insall ALSA tools.
I am testing these typec headsets on ARM8 smart phone platform. Because most x86 laptops support only 3.5mm input, I cannot test these typec devices.
Hm, but it's a USB-audio device, can't it just be plugged to a laptop...?
This problem should have something to do with host. If I remove configure_endpoints() in snd_usb_hw_params() like k5.10 behavior, this problem does not happen. It seems that to set frequency multiple times leads to headset abnormal phenomenon.
I understand that part, but it's still weird. IIUC, it happens after closing the previous stream and start a new stream, right? Then can you play two different rates on 5.10 kernel; e.g. at first call snd_pcm_prepare with a high rate and stop/close immediately. Then start quickly the stream in a normal rate again. That should issue the sample rate changes in a similar way, and this should cause the same problem, if it's really about the call of endpoint / rate setups.
Or if it's more or less a timing issue, you might try to apply the delay quirk such as QUIRK_FLAG_CTL_MSG_DELAY_XXX or QUIRK_FLAG_IFACE_DELAY.
Takashi
Thanks
On Wed, 2022-08-31 at 07:18 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 05:39:56 +0200, chihhao chen wrote:
Hi Takashi,
Yes they all show the same phenomenon : missing first sound randomly when users start playback.
Ah, that's what I misunderstood: I thought the output were completely missing.
I tried to run alsa-info.sh but got "This script requires amixer utility to continue" message.
Too bad. Any chance to install those standard ALSA tools?
For Samsung USB C Earphone UAC1 device, I tested not to set 96000(highest rate) but 48000 twice and this issue still happened.(Original behavior : set 96000 then set 48000 -> Try to set 48000 then set 48000 instead) So I think the problem might be related to setting frequency multiple times.
For Apple USB-C to 3.5mm Headphone Jack Adapter UAC3 device, I confirmed its badd_profile is UAC3_FUNCTION_SUBCLASS_HEADPHONE so it will not go into QUIRK_FLAG_VALIDATE_RATES quirk function. Besides its initialization sequence in k5.15 is to set 48000 twice and because this rate works well in k5.10, do I still need to set lower rate to test?
In that case, better to test a few other options.
But before going in that way, let's check whether the problem depends on the host or not. Which host are you testing? An ARM system? Does the problem happen with the same USB-audio device on another machine (e.g. x86 laptop)?
Takashi
Thanks
On Tue, 2022-08-30 at 10:24 +0200, Takashi Iwai wrote: > On Tue, 30 Aug 2022 10:08:51 +0200, > chihhao chen wrote: > > > > Hi Takashi, > > > > I also think it should be a firmware problem but it > > happens > > with > > many > > different devices because of new set sampling rate > > behavior > > in > > k5.15. > > > > Device 1 UAC1 > > [ 134.924359][T1000005] kworker/0:0: usb 1-1: > > [name:usbcore&]New > > USB > > device found, idVendor=04e8, idProduct=a04f, bcdDevice= > > 1.00 > > [ 134.925944][T1000005] kworker/0:0: usb 1-1: > > [name:usbcore&]New > > USB > > device strings: Mfr=1, Product=2, SerialNumber=3 > > [ 134.927338][T1000005] kworker/0:0: usb 1-1: > > [name:usbcore&]Product: > > Samsung USB C Earphone > > [ 134.928426][T1000005] kworker/0:0: usb 1-1: > > [name:usbcore&]Manufacturer: bestechnic > > [ 134.929432][T1000005] kworker/0:0: usb 1-1: > > [name:usbcore&]SerialNumber: 20160406.1 > > Does this show the same problem? If so, that's > interesting > because > UAC1 has a completely different way of setting the sample > rate. > > > Device 2 UAC3 > > [ 779.645324][T1003414] kworker/0:1: usb 1-1: > > [name:usbcore&]New > > USB > > device found, idVendor=05ac, idProduct=110a, > > bcdDevice=26.11 > > [ 779.647376][T1003414] kworker/0:1: usb 1-1: > > [name:usbcore&]New > > USB > > device strings: Mfr=1, Product=2, SerialNumber=3 > > [ 779.649492][T1003414] kworker/0:1: usb 1-1: > > [name:usbcore&]Product: > > USB-C to 3.5mm Headphone Jack Adapter > > [ 779.652262][T1003414] kworker/0:1: usb 1-1: > > [name:usbcore&]Manufacturer: Apple, Inc. > > [ 779.652273][T1003414] kworker/0:1: usb 1-1: > > [name:usbcore&]SerialNumber: DWH126301CLJKLTAF > > Device 3 > > A XiaoMi adapter but it not in my hand now. > > > > I will try to integrate k5.19 into my codebase. > > At best, please give the alsa-info.sh output from each > device. > Run the script with --no-upload option and attach the > output. > > Then try to test whether the reported highest sample rate > actually > works as-is. That is, to see whether the problem is > really > about > issuing the frequency change multiple times for different > rates, > or > it's because issuing the highest rate screws up the > device. > > And, for UAC2/3 devices, it might be worth to try some > known > quirks, > e.g. QUIRK_FLAG_VALIDATE_RATES, which was needed for MOTU > (UAC2) > devices. It's a bit 12 of quirk_flags option value. > > > Takashi > > > > > Thanks > > > > > > On Tue, 2022-08-30 at 09:02 +0200, Takashi Iwai wrote: > > > On Tue, 30 Aug 2022 08:13:44 +0200, > > > chihhao chen wrote: > > > > > > > > Hi Takashi, > > > > > > > > I tried the patch but this problem still happens. > > > > > > > > I add some logs in snd_usb_init_sample_rate() in > > > > kernel- > > > > 5.10 > > > > [ 146.260105][T1702328] writer: usb 1-1: > > > > [name:snd_usb_audio&]2:2 > > > > Set > > > > sample rate 96000, clock 0 protocol 0 > > > > [ 146.289892][T1002328] writer: usb 1-1: > > > > [name:snd_usb_audio&]2:2 > > > > Set > > > > sample rate 48000, clock 0 protocol 0 > > > > > > > > Because TinyAlsa tends to set highest rate for > > > > initialization > > > > and > > > > real > > > > rate for playback, it will still trigger two-times > > > > SAMPLING_FREQ_CONTROL USB requests. > > > > > > Then this is a firmware problem of your device. > > > The same problem would happen even with the old > > > kernel if > > > you > > > run > > > the > > > application with different sample rates. Does the > > > device > > > work > > > with > > > 96kHz at all? > > > > > > Could you give the lsusb -v output of the device, > > > too? > > > > > > > Which kernel version should I try? kernel-5.19 or? > > > > > > Yes, 5.19 should suffice. > > > > > > > > > Takashi
On Wed, 31 Aug 2022 15:16:39 +0200, chihhao chen wrote:
Hi Takashi,
Yes no error reported and data on USB bus is also complete. (Use USB analyzer to collect packets on bus and check these data.)
Hm, then it has something to do with the device firmware side...
I added delay right after find_substream_format() in snd_usb_hw_params() as follows
- first time call snd_usb_hw_params(), do nothing
- second time call snd_usb_hw_params(), delay 150ms after
find_substream_format()
I tried to set snd_usb_use_vmalloc false but this problem still happened.
OK, thanks.
On the second thought, it's good to split the existing endpoint setup to two parts, and apply the setups involving with the buffer allocation at hw_params while the USB interface setup is done at prepare. It'll reduce the unnecessary buffer re-allocation, too, so I had such a change in my mind and already cooked some time ago.
Could you try the patch below? If this actually helps for your use case, we should put more information about the good side-effect, too.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Split endpoint setups for hw_params and prepare
One of the former changes for the endpoint management was the more consistent setup of endpoints at hw_params. snd_usb_endpoint_configure() is a single function that does the full setup, and it's called from both PCM hw_params and prepare callbacks. Although the EP setup at the prepare phase is usually skipped (by checking need_setup flag), it may be still effective in some cases like suspend/resume that requires the interface setup again.
As it's a full and single setup, the invocation of snd_usb_endpoint_configure() includes not only the USB interface setup but also the buffer release and allocation. OTOH, doing the buffer release and re-allocation at PCM prepare phase is rather superfluous, and better to be only in the hw_params phase.
For those optimizations, this patch splits the endpoint setup to two phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(), to be called from hw_params and from prepare, respectively.
This changes the operation slightly, effectively moving the USB interface setup again to PCM prepare stage instead of hw_params stage, while the buffer allocation and such initializations are still done at hw_params stage.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 23 +++++++++-------------- sound/usb/endpoint.h | 6 ++++-- sound/usb/pcm.c | 14 ++++++++++---- 3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0d7b73bf7945..a42f2ce19455 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -758,7 +758,8 @@ bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, * The endpoint needs to be closed via snd_usb_endpoint_close() later. * * Note that this function doesn't configure the endpoint. The substream - * needs to set it up later via snd_usb_endpoint_configure(). + * needs to set it up later via snd_usb_endpoint_set_params() and + * snd_usb_endpoint_prepare(). */ struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, @@ -1290,12 +1291,13 @@ static int sync_ep_set_params(struct snd_usb_endpoint *ep) /* * snd_usb_endpoint_set_params: configure an snd_usb_endpoint * + * It's called either from hw_params callback. * Determine the number of URBs to be used on this endpoint. * An endpoint must be configured before it can be started. * An endpoint that is already running can not be reconfigured. */ -static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep) +int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep) { const struct audioformat *fmt = ep->cur_audiofmt; int err; @@ -1378,18 +1380,18 @@ static int init_sample_rate(struct snd_usb_audio *chip, }
/* - * snd_usb_endpoint_configure: Configure the endpoint + * snd_usb_endpoint_prepare: Prepare the endpoint * * This function sets up the EP to be fully usable state. - * It's called either from hw_params or prepare callback. + * It's called either from prepare callback. * The function checks need_setup flag, and performs nothing unless needed, * so it's safe to call this multiple times. * * This returns zero if unchanged, 1 if the configuration has changed, * or a negative error code. */ -int snd_usb_endpoint_configure(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep) +int snd_usb_endpoint_prepare(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep) { bool iface_first; int err = 0; @@ -1410,9 +1412,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock; } - err = snd_usb_endpoint_set_params(chip, ep); - if (err < 0) - goto unlock; goto done; }
@@ -1440,10 +1439,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock;
- err = snd_usb_endpoint_set_params(chip, ep); - if (err < 0) - goto unlock; - err = snd_usb_select_mode_quirk(chip, ep->cur_audiofmt); if (err < 0) goto unlock; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6a9af04cf175..e67ea28faa54 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -17,8 +17,10 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, bool is_sync_ep); void snd_usb_endpoint_close(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); -int snd_usb_endpoint_configure(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep); +int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep); +int snd_usb_endpoint_prepare(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep); int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock);
bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d45d1d7e6664..b604f7e95e82 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -443,17 +443,17 @@ static int configure_endpoints(struct snd_usb_audio *chip, if (stop_endpoints(subs, false)) sync_pending_stops(subs); if (subs->sync_endpoint) { - err = snd_usb_endpoint_configure(chip, subs->sync_endpoint); + err = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); if (err < 0) return err; } - err = snd_usb_endpoint_configure(chip, subs->data_endpoint); + err = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (err < 0) return err; snd_usb_set_format_quirk(subs, subs->cur_audiofmt); } else { if (subs->sync_endpoint) { - err = snd_usb_endpoint_configure(chip, subs->sync_endpoint); + err = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); if (err < 0) return err; } @@ -551,7 +551,13 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, subs->cur_audiofmt = fmt; mutex_unlock(&chip->mutex);
- ret = configure_endpoints(chip, subs); + if (subs->sync_endpoint) { + ret = snd_usb_endpoint_set_params(chip, subs->sync_endpoint); + if (ret < 0) + goto unlock; + } + + ret = snd_usb_endpoint_set_params(chip, subs->data_endpoint);
unlock: if (ret < 0)
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
Thanks
On Wed, 2022-08-31 at 15:40 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 15:16:39 +0200, chihhao chen wrote:
Hi Takashi,
Yes no error reported and data on USB bus is also complete. (Use USB analyzer to collect packets on bus and check these data.)
Hm, then it has something to do with the device firmware side...
I added delay right after find_substream_format() in snd_usb_hw_params() as follows
- first time call snd_usb_hw_params(), do nothing
- second time call snd_usb_hw_params(), delay 150ms after
find_substream_format()
I tried to set snd_usb_use_vmalloc false but this problem still happened.
OK, thanks.
On the second thought, it's good to split the existing endpoint setup to two parts, and apply the setups involving with the buffer allocation at hw_params while the USB interface setup is done at prepare. It'll reduce the unnecessary buffer re-allocation, too, so I had such a change in my mind and already cooked some time ago.
Could you try the patch below? If this actually helps for your use case, we should put more information about the good side-effect, too.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Split endpoint setups for hw_params and prepare
One of the former changes for the endpoint management was the more consistent setup of endpoints at hw_params. snd_usb_endpoint_configure() is a single function that does the full setup, and it's called from both PCM hw_params and prepare callbacks. Although the EP setup at the prepare phase is usually skipped (by checking need_setup flag), it may be still effective in some cases like suspend/resume that requires the interface setup again.
As it's a full and single setup, the invocation of snd_usb_endpoint_configure() includes not only the USB interface setup but also the buffer release and allocation. OTOH, doing the buffer release and re-allocation at PCM prepare phase is rather superfluous, and better to be only in the hw_params phase.
For those optimizations, this patch splits the endpoint setup to two phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(), to be called from hw_params and from prepare, respectively.
This changes the operation slightly, effectively moving the USB interface setup again to PCM prepare stage instead of hw_params stage, while the buffer allocation and such initializations are still done at hw_params stage.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/endpoint.c | 23 +++++++++-------------- sound/usb/endpoint.h | 6 ++++-- sound/usb/pcm.c | 14 ++++++++++---- 3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0d7b73bf7945..a42f2ce19455 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -758,7 +758,8 @@ bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip,
- The endpoint needs to be closed via snd_usb_endpoint_close()
later.
- Note that this function doesn't configure the endpoint. The
substream
- needs to set it up later via snd_usb_endpoint_configure().
- needs to set it up later via snd_usb_endpoint_set_params() and
*/
- snd_usb_endpoint_prepare().
struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, @@ -1290,12 +1291,13 @@ static int sync_ep_set_params(struct snd_usb_endpoint *ep) /*
- snd_usb_endpoint_set_params: configure an snd_usb_endpoint
*/
- It's called either from hw_params callback.
- Determine the number of URBs to be used on this endpoint.
- An endpoint must be configured before it can be started.
- An endpoint that is already running can not be reconfigured.
-static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep)+int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep){ const struct audioformat *fmt = ep->cur_audiofmt; int err; @@ -1378,18 +1380,18 @@ static int init_sample_rate(struct snd_usb_audio *chip, }
/*
- snd_usb_endpoint_configure: Configure the endpoint
- snd_usb_endpoint_prepare: Prepare the endpoint
- This function sets up the EP to be fully usable state.
- It's called either from hw_params or prepare callback.
- It's called either from prepare callback.
- The function checks need_setup flag, and performs nothing unless
needed,
- so it's safe to call this multiple times.
- This returns zero if unchanged, 1 if the configuration has
changed,
- or a negative error code.
*/ -int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep)+int snd_usb_endpoint_prepare(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep){ bool iface_first; int err = 0; @@ -1410,9 +1412,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock; }
err = snd_usb_endpoint_set_params(chip, ep);if (err < 0)goto unlock;@@ -1440,10 +1439,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock;
- err = snd_usb_endpoint_set_params(chip, ep);
- if (err < 0)
goto unlock;- err = snd_usb_select_mode_quirk(chip, ep->cur_audiofmt); if (err < 0) goto unlock;
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6a9af04cf175..e67ea28faa54 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -17,8 +17,10 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, bool is_sync_ep); void snd_usb_endpoint_close(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); -int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);+int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);+int snd_usb_endpoint_prepare(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock);
bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d45d1d7e6664..b604f7e95e82 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -443,17 +443,17 @@ static int configure_endpoints(struct snd_usb_audio *chip, if (stop_endpoints(subs, false)) sync_pending_stops(subs); if (subs->sync_endpoint) {
err = snd_usb_endpoint_configure(chip, subs-sync_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0) return err; }
err = snd_usb_endpoint_configure(chip, subs-data_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-data_endpoint);
if (err < 0) return err; snd_usb_set_format_quirk(subs, subs->cur_audiofmt);} else { if (subs->sync_endpoint) {
err = snd_usb_endpoint_configure(chip, subs-sync_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0) return err; }@@ -551,7 +551,13 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, subs->cur_audiofmt = fmt; mutex_unlock(&chip->mutex);
- ret = configure_endpoints(chip, subs);
- if (subs->sync_endpoint) {
ret = snd_usb_endpoint_set_params(chip, subs-sync_endpoint);
if (ret < 0)goto unlock;}
ret = snd_usb_endpoint_set_params(chip, subs->data_endpoint);
unlock: if (ret < 0)
On Thu, 01 Sep 2022 07:50:40 +0200, chihhao chen wrote:
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
OK, it's a good news. I'm going to add more information to the patch description about the regression and submit the patch.
thanks,
Takashi
Thanks
On Wed, 2022-08-31 at 15:40 +0200, Takashi Iwai wrote:
On Wed, 31 Aug 2022 15:16:39 +0200, chihhao chen wrote:
Hi Takashi,
Yes no error reported and data on USB bus is also complete. (Use USB analyzer to collect packets on bus and check these data.)
Hm, then it has something to do with the device firmware side...
I added delay right after find_substream_format() in snd_usb_hw_params() as follows
- first time call snd_usb_hw_params(), do nothing
- second time call snd_usb_hw_params(), delay 150ms after
find_substream_format()
I tried to set snd_usb_use_vmalloc false but this problem still happened.
OK, thanks.
On the second thought, it's good to split the existing endpoint setup to two parts, and apply the setups involving with the buffer allocation at hw_params while the USB interface setup is done at prepare. It'll reduce the unnecessary buffer re-allocation, too, so I had such a change in my mind and already cooked some time ago.
Could you try the patch below? If this actually helps for your use case, we should put more information about the good side-effect, too.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Split endpoint setups for hw_params and prepare
One of the former changes for the endpoint management was the more consistent setup of endpoints at hw_params. snd_usb_endpoint_configure() is a single function that does the full setup, and it's called from both PCM hw_params and prepare callbacks. Although the EP setup at the prepare phase is usually skipped (by checking need_setup flag), it may be still effective in some cases like suspend/resume that requires the interface setup again.
As it's a full and single setup, the invocation of snd_usb_endpoint_configure() includes not only the USB interface setup but also the buffer release and allocation. OTOH, doing the buffer release and re-allocation at PCM prepare phase is rather superfluous, and better to be only in the hw_params phase.
For those optimizations, this patch splits the endpoint setup to two phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(), to be called from hw_params and from prepare, respectively.
This changes the operation slightly, effectively moving the USB interface setup again to PCM prepare stage instead of hw_params stage, while the buffer allocation and such initializations are still done at hw_params stage.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/endpoint.c | 23 +++++++++-------------- sound/usb/endpoint.h | 6 ++++-- sound/usb/pcm.c | 14 ++++++++++---- 3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0d7b73bf7945..a42f2ce19455 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -758,7 +758,8 @@ bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip,
- The endpoint needs to be closed via snd_usb_endpoint_close()
later.
- Note that this function doesn't configure the endpoint. The
substream
- needs to set it up later via snd_usb_endpoint_configure().
- needs to set it up later via snd_usb_endpoint_set_params() and
*/
- snd_usb_endpoint_prepare().
struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, @@ -1290,12 +1291,13 @@ static int sync_ep_set_params(struct snd_usb_endpoint *ep) /*
- snd_usb_endpoint_set_params: configure an snd_usb_endpoint
*/
- It's called either from hw_params callback.
- Determine the number of URBs to be used on this endpoint.
- An endpoint must be configured before it can be started.
- An endpoint that is already running can not be reconfigured.
-static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep)+int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep){ const struct audioformat *fmt = ep->cur_audiofmt; int err; @@ -1378,18 +1380,18 @@ static int init_sample_rate(struct snd_usb_audio *chip, }
/*
- snd_usb_endpoint_configure: Configure the endpoint
- snd_usb_endpoint_prepare: Prepare the endpoint
- This function sets up the EP to be fully usable state.
- It's called either from hw_params or prepare callback.
- It's called either from prepare callback.
- The function checks need_setup flag, and performs nothing unless
needed,
- so it's safe to call this multiple times.
- This returns zero if unchanged, 1 if the configuration has
changed,
- or a negative error code.
*/ -int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep)+int snd_usb_endpoint_prepare(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep){ bool iface_first; int err = 0; @@ -1410,9 +1412,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock; }
err = snd_usb_endpoint_set_params(chip, ep);if (err < 0)goto unlock;@@ -1440,10 +1439,6 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip, if (err < 0) goto unlock;
- err = snd_usb_endpoint_set_params(chip, ep);
- if (err < 0)
goto unlock;- err = snd_usb_select_mode_quirk(chip, ep->cur_audiofmt); if (err < 0) goto unlock;
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6a9af04cf175..e67ea28faa54 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -17,8 +17,10 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, bool is_sync_ep); void snd_usb_endpoint_close(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); -int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);+int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);+int snd_usb_endpoint_prepare(struct snd_usb_audio *chip,
struct snd_usb_endpoint *ep);int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock);
bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d45d1d7e6664..b604f7e95e82 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -443,17 +443,17 @@ static int configure_endpoints(struct snd_usb_audio *chip, if (stop_endpoints(subs, false)) sync_pending_stops(subs); if (subs->sync_endpoint) {
err = snd_usb_endpoint_configure(chip, subs-sync_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0) return err; }
err = snd_usb_endpoint_configure(chip, subs-data_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-data_endpoint);
if (err < 0) return err; snd_usb_set_format_quirk(subs, subs->cur_audiofmt);} else { if (subs->sync_endpoint) {
err = snd_usb_endpoint_configure(chip, subs-sync_endpoint);
err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0) return err; }@@ -551,7 +551,13 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, subs->cur_audiofmt = fmt; mutex_unlock(&chip->mutex);
- ret = configure_endpoints(chip, subs);
- if (subs->sync_endpoint) {
ret = snd_usb_endpoint_set_params(chip, subs-sync_endpoint);
if (ret < 0)goto unlock;}
ret = snd_usb_endpoint_set_params(chip, subs->data_endpoint);
unlock: if (ret < 0)
On Thu, 01 Sep 2022 09:51:37 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 07:50:40 +0200, chihhao chen wrote:
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
OK, it's a good news. I'm going to add more information to the patch description about the regression and submit the patch.
Could you check whether the below cleanup patch on top of the previous one doesn't break things? I lightly tested on my devices and don't believe it would change the actual behavior, but just to be sure.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Clean up endpoint setups at PCM prepare
This patch cleans up the superfluous checks and calls for setting up the endpoints at PCM prepare callback:
- Drop stop_endpoints() and sync_pending_stops() calls; the stream is guaranteed to have been already stopped and synced at each PCM prepare call by ALSA PCM core
- Call snd_usb_endpoint_prepare() unconditionally; the check for endpoint->need_setup is done in snd_pcm_hw_endpoint_prepare() itself
- Apply snd_usb_set_format_quirk() only when the endpoint is actually set up (i.e. the return code from snd_usb_endpoint_prepare() > 0)
- Move a few lines back into snd_usb_pcm_prepare(); it's even easier to follow than a small useless function
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b604f7e95e82..4ed53a3dc922 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio *chip, } }
-static int configure_endpoints(struct snd_usb_audio *chip, - struct snd_usb_substream *subs) -{ - int err; - - if (subs->data_endpoint->need_setup) { - /* stop any running stream beforehand */ - if (stop_endpoints(subs, false)) - sync_pending_stops(subs); - if (subs->sync_endpoint) { - err = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); - if (err < 0) - return err; - } - err = snd_usb_endpoint_prepare(chip, subs->data_endpoint); - if (err < 0) - return err; - snd_usb_set_format_quirk(subs, subs->cur_audiofmt); - } else { - if (subs->sync_endpoint) { - err = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); - if (err < 0) - return err; - } - } - - return 0; -} - /* * hw_params callback * @@ -640,9 +611,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; }
- ret = configure_endpoints(chip, subs); + if (subs->sync_endpoint) { + ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); + if (ret < 0) + goto unlock; + } + + ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (ret < 0) goto unlock; + else if (ret > 0) + snd_usb_set_format_quirk(subs, subs->cur_audiofmt); + ret = 0;
/* reset the pointer */ subs->buffer_bytes = frames_to_bytes(runtime, runtime->buffer_size);
Hi Takashi,
I tested the patch and USB audio function works well. (Start and stop playback more than 20 times with Samsung USB C Earphone)
Thanks
On Thu, 2022-09-01 at 10:28 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 09:51:37 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 07:50:40 +0200, chihhao chen wrote:
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
OK, it's a good news. I'm going to add more information to the patch description about the regression and submit the patch.
Could you check whether the below cleanup patch on top of the previous one doesn't break things? I lightly tested on my devices and don't believe it would change the actual behavior, but just to be sure.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Clean up endpoint setups at PCM prepare
This patch cleans up the superfluous checks and calls for setting up the endpoints at PCM prepare callback:
Drop stop_endpoints() and sync_pending_stops() calls; the stream is guaranteed to have been already stopped and synced at each PCM prepare call by ALSA PCM core
Call snd_usb_endpoint_prepare() unconditionally; the check for endpoint->need_setup is done in snd_pcm_hw_endpoint_prepare() itself
Apply snd_usb_set_format_quirk() only when the endpoint is actually set up (i.e. the return code from snd_usb_endpoint_prepare() > 0)
Move a few lines back into snd_usb_pcm_prepare(); it's even easier to follow than a small useless function
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/pcm.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b604f7e95e82..4ed53a3dc922 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio *chip, } }
-static int configure_endpoints(struct snd_usb_audio *chip,
struct snd_usb_substream *subs)-{
- int err;
- if (subs->data_endpoint->need_setup) {
/* stop any running stream beforehand */if (stop_endpoints(subs, false))sync_pending_stops(subs);if (subs->sync_endpoint) {err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0)return err;}err = snd_usb_endpoint_prepare(chip, subs-data_endpoint);
if (err < 0)return err;snd_usb_set_format_quirk(subs, subs->cur_audiofmt);- } else {
if (subs->sync_endpoint) {err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0)return err;}- }
- return 0;
-}
/*
- hw_params callback
@@ -640,9 +611,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; }
- ret = configure_endpoints(chip, subs);
- if (subs->sync_endpoint) {
ret = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (ret < 0)goto unlock;}
ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (ret < 0) goto unlock;
else if (ret > 0)
snd_usb_set_format_quirk(subs, subs->cur_audiofmt);ret = 0;
/* reset the pointer */ subs->buffer_bytes = frames_to_bytes(runtime, runtime-
buffer_size);
On Thu, 01 Sep 2022 12:06:25 +0200, chihhao chen wrote:
Hi Takashi,
I tested the patch and USB audio function works well. (Start and stop playback more than 20 times with Samsung USB C Earphone)
Great, thanks for quick testing!
Takashi
Thanks
On Thu, 2022-09-01 at 10:28 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 09:51:37 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 07:50:40 +0200, chihhao chen wrote:
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
OK, it's a good news. I'm going to add more information to the patch description about the regression and submit the patch.
Could you check whether the below cleanup patch on top of the previous one doesn't break things? I lightly tested on my devices and don't believe it would change the actual behavior, but just to be sure.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Clean up endpoint setups at PCM prepare
This patch cleans up the superfluous checks and calls for setting up the endpoints at PCM prepare callback:
Drop stop_endpoints() and sync_pending_stops() calls; the stream is guaranteed to have been already stopped and synced at each PCM prepare call by ALSA PCM core
Call snd_usb_endpoint_prepare() unconditionally; the check for endpoint->need_setup is done in snd_pcm_hw_endpoint_prepare() itself
Apply snd_usb_set_format_quirk() only when the endpoint is actually set up (i.e. the return code from snd_usb_endpoint_prepare() > 0)
Move a few lines back into snd_usb_pcm_prepare(); it's even easier to follow than a small useless function
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/pcm.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b604f7e95e82..4ed53a3dc922 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio *chip, } }
-static int configure_endpoints(struct snd_usb_audio *chip,
struct snd_usb_substream *subs)-{
- int err;
- if (subs->data_endpoint->need_setup) {
/* stop any running stream beforehand */if (stop_endpoints(subs, false))sync_pending_stops(subs);if (subs->sync_endpoint) {err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0)return err;}err = snd_usb_endpoint_prepare(chip, subs-data_endpoint);
if (err < 0)return err;snd_usb_set_format_quirk(subs, subs->cur_audiofmt);- } else {
if (subs->sync_endpoint) {err = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (err < 0)return err;}- }
- return 0;
-}
/*
- hw_params callback
@@ -640,9 +611,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; }
- ret = configure_endpoints(chip, subs);
- if (subs->sync_endpoint) {
ret = snd_usb_endpoint_prepare(chip, subs-sync_endpoint);
if (ret < 0)goto unlock;}
ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (ret < 0) goto unlock;
else if (ret > 0)
snd_usb_set_format_quirk(subs, subs->cur_audiofmt);ret = 0;
/* reset the pointer */ subs->buffer_bytes = frames_to_bytes(runtime, runtime-
buffer_size);
Hi Takashi,
I noticed you applied related patches to kernel-6.1 and reverted the patch on kernel-5.15. Would you please patch back to kernel-5.15 because there will be more and more kernel-5.15 smart phones?
Thanks Chihhao
On Thu, 2022-09-01 at 12:25 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 12:06:25 +0200, chihhao chen wrote:
Hi Takashi,
I tested the patch and USB audio function works well. (Start and stop playback more than 20 times with Samsung USB C Earphone)
Great, thanks for quick testing!
Takashi
Thanks
On Thu, 2022-09-01 at 10:28 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 09:51:37 +0200, Takashi Iwai wrote:
On Thu, 01 Sep 2022 07:50:40 +0200, chihhao chen wrote:
Hi Takashi,
The patch fixes this problem. I tried it with "Product: Samsung USB C Earphone" and missing sound problem cannot be reproduced.
OK, it's a good news. I'm going to add more information to the patch description about the regression and submit the patch.
Could you check whether the below cleanup patch on top of the previous one doesn't break things? I lightly tested on my devices and don't believe it would change the actual behavior, but just to be sure.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Clean up endpoint setups at PCM prepare
This patch cleans up the superfluous checks and calls for setting up the endpoints at PCM prepare callback:
- Drop stop_endpoints() and sync_pending_stops() calls; the
stream is guaranteed to have been already stopped and synced at each PCM prepare call by ALSA PCM core
Call snd_usb_endpoint_prepare() unconditionally; the check for endpoint->need_setup is done in snd_pcm_hw_endpoint_prepare() itself
Apply snd_usb_set_format_quirk() only when the endpoint is
actually set up (i.e. the return code from snd_usb_endpoint_prepare() > 0)
- Move a few lines back into snd_usb_pcm_prepare(); it's even easier to follow than a small useless function
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/usb/pcm.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index b604f7e95e82..4ed53a3dc922 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -433,35 +433,6 @@ static void close_endpoints(struct snd_usb_audio *chip, } }
-static int configure_endpoints(struct snd_usb_audio *chip,
struct snd_usb_substream *subs)-{ -int err;
-if (subs->data_endpoint->need_setup) { -/* stop any running stream beforehand */ -if (stop_endpoints(subs, false)) -sync_pending_stops(subs); -if (subs->sync_endpoint) { -err = snd_usb_endpoint_prepare(chip, subs-
sync_endpoint);
-if (err < 0) -return err; -} -err = snd_usb_endpoint_prepare(chip, subs-
data_endpoint);
-if (err < 0) -return err; -snd_usb_set_format_quirk(subs, subs->cur_audiofmt); -} else { -if (subs->sync_endpoint) { -err = snd_usb_endpoint_prepare(chip, subs-
sync_endpoint);
-if (err < 0) -return err; -} -}
-return 0; -}
/*
- hw_params callback
@@ -640,9 +611,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; }
-ret = configure_endpoints(chip, subs); +if (subs->sync_endpoint) { +ret = snd_usb_endpoint_prepare(chip, subs-
sync_endpoint);
+if (ret < 0) +goto unlock; +}
+ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); if (ret < 0) goto unlock; +else if (ret > 0) +snd_usb_set_format_quirk(subs, subs->cur_audiofmt); +ret = 0;
/* reset the pointer */ subs->buffer_bytes = frames_to_bytes(runtime, runtime-
buffer_size);
************* MEDIATEK Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you!
participants (4)
-
chihhao chen -
Chihhao Chen (陳志豪)
-
Geraldo Nascimento -
Takashi Iwai