Hi,
Is the NRPACKS setting equal to the number of packets per URB ?
Best regards, Raymond
Den 29. aug.. 2012 kl. 16.14 skrev alsa-devel-request@alsa-project.org:
Send Alsa-devel mailing list submissions to alsa-devel@alsa-project.org
To subscribe or unsubscribe via the World Wide Web, visit http://mailman.alsa-project.org/mailman/listinfo/alsa-devel or, via email, send a message with subject or body 'help' to alsa-devel-request@alsa-project.org
You can reach the person managing the list at alsa-devel-owner@alsa-project.org
When replying, please edit your Subject line so it is more specific than "Re: Contents of Alsa-devel digest..."
Today's Topics:
- Re: [PATCH] ALSA: hda - Fix runtime PM accounting (Lin, Mengdong)
- Re: Logitech USB headset not working in 3.6-rc3 (Takashi Iwai)
- Re: Logitech USB headset not working in 3.6-rc3 (Daniel Mack)
- Re: [PATCH] ALSA: hda - Fix runtime PM accounting (Takashi Iwai)
- Re: Logitech USB headset not working in 3.6-rc3 (Takashi Iwai)
Message: 1 Date: Wed, 29 Aug 2012 12:19:32 +0000 From: "Lin, Mengdong" mengdong.lin@intel.com To: Takashi Iwai tiwai@suse.de Cc: "alsa-devel@alsa-project.org" alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda - Fix runtime PM accounting Message-ID: F46914AEC2663F4A9BB62374E5EEF8F81DAFDF@SHSMSX101.ccr.corp.intel.com Content-Type: text/plain; charset="us-ascii"
Now I introduced codec->epss flag that is initialized once in snd_hda_codec_new(), and let the codec driver overrides this flag. One bonus is that you can avoid the unnecessary codec parameter read at each power change.
Yes, this is better. And the patch works well for me - [PATCH 1/2] ALSA: hda - Avoid unnecessary parameter read for EPSS.
In anyway, there are a few issues:
- The refcounting gets broken after module unload. It's because we
leave the codec's pm_runtime usages after removal.
Thanks for pointing out this! But I think the following codec can introduce risk of race among multiple codecs to modify ' chip-
power_count',
because only one codec's power on/off is serialized. static void azx_power_notify(struct hda_bus *bus, struct hda_codec *codec) { ... if (codec->power_on) { pm_runtime_get_sync(&chip->pci->dev); chip->power_count++; } else { pm_runtime_put_sync(&chip->pci->dev); chip->power_count--; } }
How about calling pm_runtime_put_noidle() when freeing a codec in D0? Maybe we can add a bus notify function like this: Static void azx_codec_free_notify(struct hda_codec *codec) { If(codec->power_on) pm_runtime_put_noidle(&codec->bus->pci->dev); } And snd_hda_codec_free() can trigger this notify.
- The D3-stop-clock check isn't done properly for the codecs
providing own set_power_state ops.
I cannot test the branch 'if (codec->patch_ops.set_power_state)' in hda_set_power_state() for lack of such a codec. But the logic seems right and the other branch works well for me :-)
- Ditto for the repeated power-set sequence, the error check, etc.
In hda_sync_power_state(), if ((state & 0xff) == power_state) ... should be if ((state & 0xf0)>> 4 == power_state), because PS-Act is bits 7:4 break;
And would you please explain why changing to D3 need not wait? One of my codec does have a delay on change from D0 to D3.
Thanks Mengdong
Message: 2 Date: Wed, 29 Aug 2012 15:29:03 +0200 From: Takashi Iwai tiwai@suse.de To: Daniel Mack zonque@gmail.com Cc: Bruno Wolff III bruno@wolff.to, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Josh Boyer jwboyer@redhat.com Subject: Re: [alsa-devel] Logitech USB headset not working in 3.6-rc3 Message-ID: s5hsjb56d40.wl%tiwai@suse.de Content-Type: text/plain; charset=US-ASCII
At Wed, 29 Aug 2012 13:26:25 +0200, Daniel Mack wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 25.08.2012 14:17, Josh Boyer wrote:
On Sat, Aug 25, 2012 at 02:13:58PM +0200, Daniel Mack wrote:
On 25.08.2012 14:07, Bruno Wolff III wrote:
On Sat, Aug 25, 2012 at 14:02:51 +0200, Daniel Mack zonque@gmail.com wrote:
Can you revert commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream") and see if that
I can try that, but it takes a long time to build a new kernel on my old hardware.
helps? If not, can you summarize again which kernels still work for you and which don't?
The latest kernel that works is 3.6.0-0.rc2.git1.2.fc18. The earliest that doesn't work is 3.6.0-0.rc2.git2.1.fc18.
The report you sent doesn't look like it could be caused by e9ba389c5. It fixes a kernel Ooops. But as it is the only relevant patch in that area, it would be interesting if reverting it fixes anything.
Yep, agreed. If this revert kernel doesn't work, we're likely down to a git bisect, Bruno.
Btw - thanks a lot for testing -rc kernels, much appreciated!
Indeed!
Could you please try this patch on top of Takashi's? Thanks again!
Daniel
[2 0001-ALSA-snd-usb-Fix-URB-cancellation-at-stream-start.patch <text/x-patch (7bit)>]
From 6617bb2463ae3fad21390b59cc2a71f96b9e9ca8 Mon Sep 17 00:00:00 2001
From: Daniel Mack zonque@gmail.com Date: Wed, 29 Aug 2012 13:17:05 +0200 Subject: [PATCH] ALSA: snd-usb: Fix URB cancellation at stream start
Commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream") fixed a scheduling-while-atomic bug that happened when snd_usb_endpoint_start was called from the trigger callback, which is an atmic context. However, the patch breaks the idea of the endpoints reference counting, which is the reason why the driver has been refactored lately.
Revert that commit and let snd_usb_endpoint_start() take care of the URB cancellation again. As this function is called from both atomic and non-atomic context, add a flag to denote whether the function may sleep.
Signed-off-by: Daniel Mack zonque@gmail.com Cc: stable@kernel.org [3.5+]
sound/usb/endpoint.c | 10 ++++++++-- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 13 +++++-------- 3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index c411812..678456c 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -799,7 +799,9 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, /**
- snd_usb_endpoint_start: start an snd_usb_endpoint
- @ep: the endpoint to start
- @ep: the endpoint to start
- @can_sleep: flag indicating whether the operation is executed in
non-atomic context
- A call to this function will increment the use count of the
endpoint.
- In case it is not already running, the URBs for this endpoint
will be @@ -809,7 +811,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
- Returns an error if the URB submission failed, 0 in all other
cases. */ -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep) { int err; unsigned int i; @@ -821,6 +823,10 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (++ep->use_count != 1) return 0;
- /* just to be sure */
- deactivate_urbs(ep, 0, can_sleep);
- wait_clear_urbs(ep);
It'd be safer to protect the call of wait_clear_urbs() when can_sleep=0.
thanks,
Takashi
Message: 3 Date: Wed, 29 Aug 2012 15:32:34 +0200 From: Daniel Mack zonque@gmail.com To: Takashi Iwai tiwai@suse.de Cc: Bruno Wolff III bruno@wolff.to, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Josh Boyer jwboyer@redhat.com Subject: Re: [alsa-devel] Logitech USB headset not working in 3.6-rc3 Message-ID: 503E19F2.3080302@gmail.com Content-Type: text/plain; charset="iso-8859-1"
On 29.08.2012 15:29, Takashi Iwai wrote:
At Wed, 29 Aug 2012 13:26:25 +0200, Daniel Mack wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 25.08.2012 14:17, Josh Boyer wrote:
On Sat, Aug 25, 2012 at 02:13:58PM +0200, Daniel Mack wrote:
On 25.08.2012 14:07, Bruno Wolff III wrote:
On Sat, Aug 25, 2012 at 14:02:51 +0200, Daniel Mack zonque@gmail.com wrote: > > Can you revert commit e9ba389c5 ("ALSA: usb-audio: Fix > scheduling-while-atomic bug in PCM capture stream") and see if > that
I can try that, but it takes a long time to build a new kernel on my old hardware.
> helps? If not, can you summarize again which kernels still > work for you > and which don't?
The latest kernel that works is 3.6.0-0.rc2.git1.2.fc18. The earliest that doesn't work is 3.6.0-0.rc2.git2.1.fc18.
The report you sent doesn't look like it could be caused by e9ba389c5. It fixes a kernel Ooops. But as it is the only relevant patch in that area, it would be interesting if reverting it fixes anything.
Yep, agreed. If this revert kernel doesn't work, we're likely down to a git bisect, Bruno.
Btw - thanks a lot for testing -rc kernels, much appreciated!
Indeed!
Could you please try this patch on top of Takashi's? Thanks again!
Daniel
[2 0001-ALSA-snd-usb-Fix-URB-cancellation-at-stream-start.patch <text/x-patch (7bit)>]
From 6617bb2463ae3fad21390b59cc2a71f96b9e9ca8 Mon Sep 17 00:00:00 2001
From: Daniel Mack zonque@gmail.com Date: Wed, 29 Aug 2012 13:17:05 +0200 Subject: [PATCH] ALSA: snd-usb: Fix URB cancellation at stream start
Commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream") fixed a scheduling-while-atomic bug that happened when snd_usb_endpoint_start was called from the trigger callback, which is an atmic context. However, the patch breaks the idea of the endpoints reference counting, which is the reason why the driver has been refactored lately.
Revert that commit and let snd_usb_endpoint_start() take care of the URB cancellation again. As this function is called from both atomic and non-atomic context, add a flag to denote whether the function may sleep.
Signed-off-by: Daniel Mack zonque@gmail.com Cc: stable@kernel.org [3.5+]
sound/usb/endpoint.c | 10 ++++++++-- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 13 +++++-------- 3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index c411812..678456c 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -799,7 +799,9 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, /**
- snd_usb_endpoint_start: start an snd_usb_endpoint
- @ep: the endpoint to start
- @ep: the endpoint to start
- @can_sleep: flag indicating whether the operation is executed in
non-atomic context
- A call to this function will increment the use count of the
endpoint.
- In case it is not already running, the URBs for this endpoint
will be @@ -809,7 +811,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
- Returns an error if the URB submission failed, 0 in all other
cases. */ -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep) { int err; unsigned int i; @@ -821,6 +823,10 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (++ep->use_count != 1) return 0;
- /* just to be sure */
- deactivate_urbs(ep, 0, can_sleep);
- wait_clear_urbs(ep);
It'd be safer to protect the call of wait_clear_urbs() when can_sleep=0.
Right. New patch attached.