From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] Add M2Tech hiFace USB-SPDIF driver Date: Tue, 12 Feb 2013 16:29:56 +0100 Message-ID: References: <1360537887-9427-1-git-send-email-ao2@amarulasolutions.com> <5118B186.1020801@ladisch.de> <20130212133521.39628e4aef556560b8b14dd8@studenti.unina.it> <511A51BF.6000602@ladisch.de> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id A1D95264FBA for ; Tue, 12 Feb 2013 16:29:57 +0100 (CET) In-Reply-To: <511A51BF.6000602@ladisch.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Clemens Ladisch Cc: alsa-devel@alsa-project.org, Antonio Ospite , fchecconi@gmail.com, Pietro Cipriano , alberto@amarulasolutions.com, Michael Trimarchi , Antonio Ospite List-Id: alsa-devel@alsa-project.org At Tue, 12 Feb 2013 15:29:19 +0100, Clemens Ladisch wrote: > > >>> + snd_pcm_stop(rt->playback.instance, > >>> + SNDRV_PCM_STATE_XRUN); > >> > >> This requres locking the stream with something like > >> snd_pcm_stream_lock_irq(). > > > > OK, but would you mind elaborating a little more why this is needed? > > > > I do not see other drivers doing that, > > sound/firewire/{amdtp,isight}.c do. I've never said that all those > other drivers would be correct. (And you have to be careful to avoid > deadlocks.) > > > and BTW I see also that some other drivers not calling snd_pcm_stop() > > at all, e.g. it is commented in sound/usb/endpoint.c::snd_complete_urb(). > > It's commented because it would crash. That driver isn't in a very > good state regarding state changes. The problem is that you can't kill the urb itself gracefully while in the complete callback. But a patch like below could work a little bit better, although it doesn't notify the XRUN at that moment. Takashi --- diff --git a/sound/usb/card.h b/sound/usb/card.h index 8a751b4..d98e7bc 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -118,6 +118,7 @@ struct snd_usb_substream { unsigned int fmt_type; /* USB audio format type (1-3) */ unsigned int running: 1; /* running status */ + unsigned int xrun:1; unsigned int hwptr_done; /* processed byte position in the buffer */ unsigned int transfer_done; /* processed frames since last period update */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 21049b8..e52aee9 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -357,6 +357,9 @@ static void snd_complete_urb(struct urb *urb) ep->chip->shutdown)) /* device disconnected */ goto exit_clear; + if (ep->data_subs && ep->data_subs->xrun) + goto exit_clear; + if (usb_pipeout(ep->pipe)) { retire_outbound_urb(ep, ctx); /* can be stopped during retire callback */ @@ -389,7 +392,8 @@ static void snd_complete_urb(struct urb *urb) return; snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err); - //snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); + if (ep->data_subs) + ep->data_subs->xrun = 1; exit_clear: clear_bit(ctx->index, &ep->active_mask); diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index f94397b..6be6662 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -79,7 +79,7 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream unsigned int hwptr_done; subs = (struct snd_usb_substream *)substream->runtime->private_data; - if (subs->stream->chip->shutdown) + if (subs->xrun || subs->stream->chip->shutdown) return SNDRV_PCM_POS_XRUN; spin_lock(&subs->lock); hwptr_done = subs->hwptr_done; @@ -726,6 +726,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->transfer_done = 0; subs->last_delay = 0; subs->last_frame_number = 0; + subs->xrun = 0; runtime->delay = 0; /* for playback, submit the URBs now; otherwise, the first hwptr_done