This reverts commit 03a8b0df757f1beb21ba1626e23ca7412e48b525.
This reverts commit c11117b634f4f832c4420d3cf41c44227f140ce1.
Pipewire and PulseAudio start devices with 44.1khz before changing them
to 48khz (or something different). By locking the rate, daemons are
unable to enumerate possible rates, and so they never change them to a
more optimal rate. This revert patch should allow 48khz audio again.
Fixes: c11117b634f4 ("ALSA: usb-audio: Refcount multiple accesses on the single clock")
Cc: [email protected]
Cc: Takashi Iwai <[email protected]>
Cc: Niklāvs Koļesņikovs <[email protected]>
Cc: Wim Taymans <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Link: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2620
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
sound/usb/card.c | 1 -
sound/usb/card.h | 3 +-
sound/usb/endpoint.c | 90 +++++---------------------------------------
sound/usb/usbaudio.h | 1 -
4 files changed, 10 insertions(+), 85 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c
index d356743de2ff..12bca2e85505 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -643,7 +643,6 @@ static int snd_usb_audio_create(struct usb_interface *intf,
INIT_LIST_HEAD(&chip->pcm_list);
INIT_LIST_HEAD(&chip->ep_list);
INIT_LIST_HEAD(&chip->iface_ref_list);
- INIT_LIST_HEAD(&chip->clock_ref_list);
INIT_LIST_HEAD(&chip->midi_list);
INIT_LIST_HEAD(&chip->mixer_list);
diff --git a/sound/usb/card.h b/sound/usb/card.h
index ca75f2206170..87f042d06ce0 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -44,7 +44,6 @@ struct audioformat {
struct snd_usb_substream;
struct snd_usb_iface_ref;
-struct snd_usb_clock_ref;
struct snd_usb_endpoint;
struct snd_usb_power_domain;
@@ -63,7 +62,6 @@ struct snd_urb_ctx {
struct snd_usb_endpoint {
struct snd_usb_audio *chip;
struct snd_usb_iface_ref *iface_ref;
- struct snd_usb_clock_ref *clock_ref;
int opened; /* open refcount; protect with chip->mutex */
atomic_t running; /* running status */
@@ -140,6 +138,7 @@ struct snd_usb_endpoint {
unsigned int cur_period_frames;
unsigned int cur_period_bytes;
unsigned int cur_buffer_periods;
+ unsigned char cur_clock;
spinlock_t lock;
struct list_head list;
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 0d7b73bf7945..0f5e9224a799 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -35,14 +35,6 @@ struct snd_usb_iface_ref {
struct list_head list;
};
-/* clock refcounting */
-struct snd_usb_clock_ref {
- unsigned char clock;
- atomic_t locked;
- int rate;
- struct list_head list;
-};
-
/*
* snd_usb_endpoint is a model that abstracts everything related to an
* USB endpoint and its streaming.
@@ -599,25 +591,6 @@ iface_ref_find(struct snd_usb_audio *chip, int iface)
return ip;
}
-/* Similarly, a refcount object for clock */
-static struct snd_usb_clock_ref *
-clock_ref_find(struct snd_usb_audio *chip, int clock)
-{
- struct snd_usb_clock_ref *ref;
-
- list_for_each_entry(ref, &chip->clock_ref_list, list)
- if (ref->clock == clock)
- return ref;
-
- ref = kzalloc(sizeof(*ref), GFP_KERNEL);
- if (!ref)
- return NULL;
- ref->clock = clock;
- atomic_set(&ref->locked, 0);
- list_add_tail(&ref->list, &chip->clock_ref_list);
- return ref;
-}
-
/*
* Get the existing endpoint object corresponding EP
* Returns NULL if not present.
@@ -795,14 +768,6 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip,
goto unlock;
}
- if (fp->protocol != UAC_VERSION_1) {
- ep->clock_ref = clock_ref_find(chip, fp->clock);
- if (!ep->clock_ref) {
- ep = NULL;
- goto unlock;
- }
- }
-
ep->cur_audiofmt = fp;
ep->cur_channels = fp->channels;
ep->cur_rate = params_rate(params);
@@ -812,6 +777,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip,
ep->cur_period_frames = params_period_size(params);
ep->cur_period_bytes = ep->cur_period_frames * ep->cur_frame_bytes;
ep->cur_buffer_periods = params_periods(params);
+ ep->cur_clock = fp->clock;
if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC)
endpoint_set_syncinterval(chip, ep);
@@ -928,8 +894,8 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip,
ep->altsetting = 0;
ep->cur_audiofmt = NULL;
ep->cur_rate = 0;
+ ep->cur_clock = 0;
ep->iface_ref = NULL;
- ep->clock_ref = NULL;
usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num);
}
mutex_unlock(&chip->mutex);
@@ -941,8 +907,6 @@ void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep)
ep->need_setup = true;
if (ep->iface_ref)
ep->iface_ref->need_setup = true;
- if (ep->clock_ref)
- ep->clock_ref->rate = 0;
}
/*
@@ -1350,33 +1314,6 @@ static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
return 0;
}
-static int init_sample_rate(struct snd_usb_audio *chip,
- struct snd_usb_endpoint *ep)
-{
- struct snd_usb_clock_ref *clock = ep->clock_ref;
- int err;
-
- if (clock) {
- if (atomic_read(&clock->locked))
- return 0;
- if (clock->rate == ep->cur_rate)
- return 0;
- if (clock->rate && clock->rate != ep->cur_rate) {
- usb_audio_dbg(chip, "Mismatched sample rate %d vs %d for EP 0x%x\n",
- clock->rate, ep->cur_rate, ep->ep_num);
- return -EINVAL;
- }
- }
-
- err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate);
- if (err < 0)
- return err;
-
- if (clock)
- clock->rate = ep->cur_rate;
- return 0;
-}
-
/*
* snd_usb_endpoint_configure: Configure the endpoint
*
@@ -1406,7 +1343,8 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
* to update at each EP configuration
*/
if (ep->cur_audiofmt->protocol == UAC_VERSION_1) {
- err = init_sample_rate(chip, ep);
+ err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt,
+ ep->cur_rate);
if (err < 0)
goto unlock;
}
@@ -1436,7 +1374,7 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
if (err < 0)
goto unlock;
- err = init_sample_rate(chip, ep);
+ err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate);
if (err < 0)
goto unlock;
@@ -1469,15 +1407,15 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
/* get the current rate set to the given clock by any endpoint */
int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock)
{
- struct snd_usb_clock_ref *ref;
+ struct snd_usb_endpoint *ep;
int rate = 0;
if (!clock)
return 0;
mutex_lock(&chip->mutex);
- list_for_each_entry(ref, &chip->clock_ref_list, list) {
- if (ref->clock == clock) {
- rate = ref->rate;
+ list_for_each_entry(ep, &chip->ep_list, list) {
+ if (ep->cur_clock == clock && ep->cur_rate) {
+ rate = ep->cur_rate;
break;
}
}
@@ -1518,9 +1456,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
if (atomic_inc_return(&ep->running) != 1)
return 0;
- if (ep->clock_ref)
- atomic_inc(&ep->clock_ref->locked);
-
ep->active_mask = 0;
ep->unlink_mask = 0;
ep->phase = 0;
@@ -1630,9 +1565,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending)
if (ep->sync_source)
WRITE_ONCE(ep->sync_source->sync_sink, NULL);
stop_urbs(ep, false, keep_pending);
- if (ep->clock_ref)
- if (!atomic_dec_return(&ep->clock_ref->locked))
- ep->clock_ref->rate = 0;
}
}
@@ -1659,16 +1591,12 @@ void snd_usb_endpoint_free_all(struct snd_usb_audio *chip)
{
struct snd_usb_endpoint *ep, *en;
struct snd_usb_iface_ref *ip, *in;
- struct snd_usb_clock_ref *cp, *cn;
list_for_each_entry_safe(ep, en, &chip->ep_list, list)
kfree(ep);
list_for_each_entry_safe(ip, in, &chip->iface_ref_list, list)
kfree(ip);
-
- list_for_each_entry_safe(cp, cn, &chip->clock_ref_list, list)
- kfree(cp);
}
/*
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index ffbb4b0d09a0..044cd7ab27cb 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -45,7 +45,6 @@ struct snd_usb_audio {
struct list_head pcm_list; /* list of pcm streams */
struct list_head ep_list; /* list of audio-related endpoints */
struct list_head iface_ref_list; /* list of interface refcounts */
- struct list_head clock_ref_list; /* list of clock refcounts */
int pcm_devs;
struct list_head midi_list; /* list of midi interfaces */
--
2.37.3
On Mon, 05 Sep 2022 12:14:03 +0200,
Jason A. Donenfeld wrote:
>
> This reverts commit 03a8b0df757f1beb21ba1626e23ca7412e48b525.
> This reverts commit c11117b634f4f832c4420d3cf41c44227f140ce1.
>
> Pipewire and PulseAudio start devices with 44.1khz before changing them
> to 48khz (or something different). By locking the rate, daemons are
> unable to enumerate possible rates, and so they never change them to a
> more optimal rate. This revert patch should allow 48khz audio again.
Well, in that case, the revert is no right solution, IMO.
If the patch caused a problem, it means that the application tries to
change the rate while it's being still running by another. If it
worked, it worked just casually without noticing the bad behavior.
That said, changing the rate for a tied stream with a different rate
while another is actually running is anyway no-go. Maybe it should
have better PCM hw constraint for covering this. We need to check
more details.
When you load snd-usb-audio with dyndbg=+p option, does it show the
new error message "Mismatched sample rate xxx"?
thanks,
Takashi
>
> Fixes: c11117b634f4 ("ALSA: usb-audio: Refcount multiple accesses on the single clock")
> Cc: [email protected]
> Cc: Takashi Iwai <[email protected]>
> Cc: Nikl?vs Ko?es?ikovs <[email protected]>
> Cc: Wim Taymans <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]/
> Link: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2620
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> sound/usb/card.c | 1 -
> sound/usb/card.h | 3 +-
> sound/usb/endpoint.c | 90 +++++---------------------------------------
> sound/usb/usbaudio.h | 1 -
> 4 files changed, 10 insertions(+), 85 deletions(-)
>
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index d356743de2ff..12bca2e85505 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -643,7 +643,6 @@ static int snd_usb_audio_create(struct usb_interface *intf,
> INIT_LIST_HEAD(&chip->pcm_list);
> INIT_LIST_HEAD(&chip->ep_list);
> INIT_LIST_HEAD(&chip->iface_ref_list);
> - INIT_LIST_HEAD(&chip->clock_ref_list);
> INIT_LIST_HEAD(&chip->midi_list);
> INIT_LIST_HEAD(&chip->mixer_list);
>
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index ca75f2206170..87f042d06ce0 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -44,7 +44,6 @@ struct audioformat {
>
> struct snd_usb_substream;
> struct snd_usb_iface_ref;
> -struct snd_usb_clock_ref;
> struct snd_usb_endpoint;
> struct snd_usb_power_domain;
>
> @@ -63,7 +62,6 @@ struct snd_urb_ctx {
> struct snd_usb_endpoint {
> struct snd_usb_audio *chip;
> struct snd_usb_iface_ref *iface_ref;
> - struct snd_usb_clock_ref *clock_ref;
>
> int opened; /* open refcount; protect with chip->mutex */
> atomic_t running; /* running status */
> @@ -140,6 +138,7 @@ struct snd_usb_endpoint {
> unsigned int cur_period_frames;
> unsigned int cur_period_bytes;
> unsigned int cur_buffer_periods;
> + unsigned char cur_clock;
>
> spinlock_t lock;
> struct list_head list;
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 0d7b73bf7945..0f5e9224a799 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -35,14 +35,6 @@ struct snd_usb_iface_ref {
> struct list_head list;
> };
>
> -/* clock refcounting */
> -struct snd_usb_clock_ref {
> - unsigned char clock;
> - atomic_t locked;
> - int rate;
> - struct list_head list;
> -};
> -
> /*
> * snd_usb_endpoint is a model that abstracts everything related to an
> * USB endpoint and its streaming.
> @@ -599,25 +591,6 @@ iface_ref_find(struct snd_usb_audio *chip, int iface)
> return ip;
> }
>
> -/* Similarly, a refcount object for clock */
> -static struct snd_usb_clock_ref *
> -clock_ref_find(struct snd_usb_audio *chip, int clock)
> -{
> - struct snd_usb_clock_ref *ref;
> -
> - list_for_each_entry(ref, &chip->clock_ref_list, list)
> - if (ref->clock == clock)
> - return ref;
> -
> - ref = kzalloc(sizeof(*ref), GFP_KERNEL);
> - if (!ref)
> - return NULL;
> - ref->clock = clock;
> - atomic_set(&ref->locked, 0);
> - list_add_tail(&ref->list, &chip->clock_ref_list);
> - return ref;
> -}
> -
> /*
> * Get the existing endpoint object corresponding EP
> * Returns NULL if not present.
> @@ -795,14 +768,6 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip,
> goto unlock;
> }
>
> - if (fp->protocol != UAC_VERSION_1) {
> - ep->clock_ref = clock_ref_find(chip, fp->clock);
> - if (!ep->clock_ref) {
> - ep = NULL;
> - goto unlock;
> - }
> - }
> -
> ep->cur_audiofmt = fp;
> ep->cur_channels = fp->channels;
> ep->cur_rate = params_rate(params);
> @@ -812,6 +777,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip,
> ep->cur_period_frames = params_period_size(params);
> ep->cur_period_bytes = ep->cur_period_frames * ep->cur_frame_bytes;
> ep->cur_buffer_periods = params_periods(params);
> + ep->cur_clock = fp->clock;
>
> if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC)
> endpoint_set_syncinterval(chip, ep);
> @@ -928,8 +894,8 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip,
> ep->altsetting = 0;
> ep->cur_audiofmt = NULL;
> ep->cur_rate = 0;
> + ep->cur_clock = 0;
> ep->iface_ref = NULL;
> - ep->clock_ref = NULL;
> usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num);
> }
> mutex_unlock(&chip->mutex);
> @@ -941,8 +907,6 @@ void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep)
> ep->need_setup = true;
> if (ep->iface_ref)
> ep->iface_ref->need_setup = true;
> - if (ep->clock_ref)
> - ep->clock_ref->rate = 0;
> }
>
> /*
> @@ -1350,33 +1314,6 @@ static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
> return 0;
> }
>
> -static int init_sample_rate(struct snd_usb_audio *chip,
> - struct snd_usb_endpoint *ep)
> -{
> - struct snd_usb_clock_ref *clock = ep->clock_ref;
> - int err;
> -
> - if (clock) {
> - if (atomic_read(&clock->locked))
> - return 0;
> - if (clock->rate == ep->cur_rate)
> - return 0;
> - if (clock->rate && clock->rate != ep->cur_rate) {
> - usb_audio_dbg(chip, "Mismatched sample rate %d vs %d for EP 0x%x\n",
> - clock->rate, ep->cur_rate, ep->ep_num);
> - return -EINVAL;
> - }
> - }
> -
> - err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate);
> - if (err < 0)
> - return err;
> -
> - if (clock)
> - clock->rate = ep->cur_rate;
> - return 0;
> -}
> -
> /*
> * snd_usb_endpoint_configure: Configure the endpoint
> *
> @@ -1406,7 +1343,8 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
> * to update at each EP configuration
> */
> if (ep->cur_audiofmt->protocol == UAC_VERSION_1) {
> - err = init_sample_rate(chip, ep);
> + err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt,
> + ep->cur_rate);
> if (err < 0)
> goto unlock;
> }
> @@ -1436,7 +1374,7 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
> if (err < 0)
> goto unlock;
>
> - err = init_sample_rate(chip, ep);
> + err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate);
> if (err < 0)
> goto unlock;
>
> @@ -1469,15 +1407,15 @@ int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
> /* get the current rate set to the given clock by any endpoint */
> int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock)
> {
> - struct snd_usb_clock_ref *ref;
> + struct snd_usb_endpoint *ep;
> int rate = 0;
>
> if (!clock)
> return 0;
> mutex_lock(&chip->mutex);
> - list_for_each_entry(ref, &chip->clock_ref_list, list) {
> - if (ref->clock == clock) {
> - rate = ref->rate;
> + list_for_each_entry(ep, &chip->ep_list, list) {
> + if (ep->cur_clock == clock && ep->cur_rate) {
> + rate = ep->cur_rate;
> break;
> }
> }
> @@ -1518,9 +1456,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
> if (atomic_inc_return(&ep->running) != 1)
> return 0;
>
> - if (ep->clock_ref)
> - atomic_inc(&ep->clock_ref->locked);
> -
> ep->active_mask = 0;
> ep->unlink_mask = 0;
> ep->phase = 0;
> @@ -1630,9 +1565,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending)
> if (ep->sync_source)
> WRITE_ONCE(ep->sync_source->sync_sink, NULL);
> stop_urbs(ep, false, keep_pending);
> - if (ep->clock_ref)
> - if (!atomic_dec_return(&ep->clock_ref->locked))
> - ep->clock_ref->rate = 0;
> }
> }
>
> @@ -1659,16 +1591,12 @@ void snd_usb_endpoint_free_all(struct snd_usb_audio *chip)
> {
> struct snd_usb_endpoint *ep, *en;
> struct snd_usb_iface_ref *ip, *in;
> - struct snd_usb_clock_ref *cp, *cn;
>
> list_for_each_entry_safe(ep, en, &chip->ep_list, list)
> kfree(ep);
>
> list_for_each_entry_safe(ip, in, &chip->iface_ref_list, list)
> kfree(ip);
> -
> - list_for_each_entry_safe(cp, cn, &chip->clock_ref_list, list)
> - kfree(cp);
> }
>
> /*
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index ffbb4b0d09a0..044cd7ab27cb 100644
> --- a/sound/usb/usbaudio.h
> +++ b/sound/usb/usbaudio.h
> @@ -45,7 +45,6 @@ struct snd_usb_audio {
> struct list_head pcm_list; /* list of pcm streams */
> struct list_head ep_list; /* list of audio-related endpoints */
> struct list_head iface_ref_list; /* list of interface refcounts */
> - struct list_head clock_ref_list; /* list of clock refcounts */
> int pcm_devs;
>
> struct list_head midi_list; /* list of midi interfaces */
> --
> 2.37.3
>
On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
>
> On Mon, 05 Sep 2022 12:14:03 +0200,
> Jason A. Donenfeld wrote:
> >
> > This reverts commit 03a8b0df757f1beb21ba1626e23ca7412e48b525.
> > This reverts commit c11117b634f4f832c4420d3cf41c44227f140ce1.
> >
> > Pipewire and PulseAudio start devices with 44.1khz before changing them
> > to 48khz (or something different). By locking the rate, daemons are
> > unable to enumerate possible rates, and so they never change them to a
> > more optimal rate. This revert patch should allow 48khz audio again.
>
> Well, in that case, the revert is no right solution, IMO.
> If the patch caused a problem, it means that the application tries to
> change the rate while it's being still running by another. If it
> worked, it worked just casually without noticing the bad behavior.
Not sure this is really what's happening. I think the issue is that
alsa reports that the device only supports a limited set of rates.
Pipewire then doesn't see 48khz, so it doesn't try to
stop,reclock,start.
Maybe Wim or Niklavs can provide more info about this.
Jason
On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
> When you load snd-usb-audio with dyndbg=+p option, does it show the
> new error message "Mismatched sample rate xxx"?
No.
On Mon, 05 Sep 2022 14:06:45 +0200,
Jason A. Donenfeld wrote:
>
> On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
> >
> > On Mon, 05 Sep 2022 12:14:03 +0200,
> > Jason A. Donenfeld wrote:
> > >
> > > This reverts commit 03a8b0df757f1beb21ba1626e23ca7412e48b525.
> > > This reverts commit c11117b634f4f832c4420d3cf41c44227f140ce1.
> > >
> > > Pipewire and PulseAudio start devices with 44.1khz before changing them
> > > to 48khz (or something different). By locking the rate, daemons are
> > > unable to enumerate possible rates, and so they never change them to a
> > > more optimal rate. This revert patch should allow 48khz audio again.
> >
> > Well, in that case, the revert is no right solution, IMO.
> > If the patch caused a problem, it means that the application tries to
> > change the rate while it's being still running by another. If it
> > worked, it worked just casually without noticing the bad behavior.
>
> Not sure this is really what's happening. I think the issue is that
> alsa reports that the device only supports a limited set of rates.
This patch doesn't change the "report" mechanism. Instead what this
patch does is to bail out as an error if you try to change the rate of
a coupled stream while another stream is already running.
> Pipewire then doesn't see 48khz, so it doesn't try to
> stop,reclock,start.
>
> Maybe Wim or Niklavs can provide more info about this.
More information is appreciated :)
Takashi
On Mon, 05 Sep 2022 14:16:39 +0200,
Jason A. Donenfeld wrote:
>
> On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
> > When you load snd-usb-audio with dyndbg=+p option, does it show the
> > new error message "Mismatched sample rate xxx"?
>
> No.
What about the patch below?
Takashi
-- 8< --
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -925,6 +925,8 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip,
endpoint_set_interface(chip, ep, false);
if (!--ep->opened) {
+ if (ep->clock_ref && !atomic_read(&ep->clock_ref->locked))
+ ep->clock_ref->rate = 0;
ep->iface = 0;
ep->altsetting = 0;
ep->cur_audiofmt = NULL;
On Wed, 07 Sep 2022 11:28:54 +0200,
Jason A. Donenfeld wrote:
>
> On Wed, Sep 07, 2022 at 10:00:46AM +0200, Takashi Iwai wrote:
> > On Mon, 05 Sep 2022 14:16:39 +0200,
> > Jason A. Donenfeld wrote:
> > >
> > > On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
> > > > When you load snd-usb-audio with dyndbg=+p option, does it show the
> > > > new error message "Mismatched sample rate xxx"?
> > >
> > > No.
> >
> > What about the patch below?
> >
> >
> > Takashi
> >
> > -- 8< --
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -925,6 +925,8 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip,
> > endpoint_set_interface(chip, ep, false);
> >
> > if (!--ep->opened) {
> > + if (ep->clock_ref && !atomic_read(&ep->clock_ref->locked))
> > + ep->clock_ref->rate = 0;
> > ep->iface = 0;
> > ep->altsetting = 0;
> > ep->cur_audiofmt = NULL;
>
> I think this works.
OK, thanks.
If this patch fixes, the problem is that the behavior of the
applications that do only PCM prepare without actually starting, then
closes. This left the last set rate unexpectedly.
I'm going to write up the proper patch.
Takashi
On Wed, Sep 07, 2022 at 10:00:46AM +0200, Takashi Iwai wrote:
> On Mon, 05 Sep 2022 14:16:39 +0200,
> Jason A. Donenfeld wrote:
> >
> > On Mon, Sep 5, 2022 at 1:44 PM Takashi Iwai <[email protected]> wrote:
> > > When you load snd-usb-audio with dyndbg=+p option, does it show the
> > > new error message "Mismatched sample rate xxx"?
> >
> > No.
>
> What about the patch below?
>
>
> Takashi
>
> -- 8< --
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -925,6 +925,8 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip,
> endpoint_set_interface(chip, ep, false);
>
> if (!--ep->opened) {
> + if (ep->clock_ref && !atomic_read(&ep->clock_ref->locked))
> + ep->clock_ref->rate = 0;
> ep->iface = 0;
> ep->altsetting = 0;
> ep->cur_audiofmt = NULL;
I think this works.
Niklāvs - can you give it a try to and confirm?
Jason