diff mbox series

[v2,07/11] usb: gadget: u_audio: Stopping PCM substream at capture/playback stop

Message ID 20211220211130.88590-8-pavel.hofman@ivitera.com
State Superseded
Headers show
Series usb: gadget: audio: Multiple rates, dyn. bInterval | expand

Commit Message

Pavel Hofman Dec. 20, 2021, 9:11 p.m. UTC
When the USB host stops capture/playback, the corresponding PCM
substream (if activated and running) is stopped and drained.

Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
---
 drivers/usb/gadget/function/u_audio.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

John Keeping Dec. 21, 2021, 12:18 p.m. UTC | #1
On Mon, Dec 20, 2021 at 10:11:26PM +0100, Pavel Hofman wrote:
> When the USB host stops capture/playback, the corresponding PCM
> substream (if activated and running) is stopped and drained.
> 
> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
> ---
>  drivers/usb/gadget/function/u_audio.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index a6293415c071..9dbce51c2eb7 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -544,6 +544,20 @@ static void set_reported_srate(struct uac_rtd_params *prm, int srate)
>  	}
>  }
>  
> +static void stop_substream(struct uac_rtd_params *prm)
> +{
> +	unsigned long _flags;
> +	struct snd_pcm_substream *substream;
> +
> +	substream = prm->ss;
> +	if (substream) {
> +		snd_pcm_stream_lock_irqsave(substream, _flags);
> +		if (snd_pcm_running(substream))
> +			snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);

I'm not sure if this is right (and the series should probably be CC'd to
alsa-devel to check the audio side of this).

DRAINING seems to be right for capture, but for playback should this end
up in state SETUP?  Does this need to handle resuming a paused stream
like snd_pcm_drain() does?

> +		snd_pcm_stream_unlock_irqrestore(substream, _flags);
> +	}
> +}
> +
>  int u_audio_start_capture(struct g_audio *audio_dev)
>  {
>  	struct snd_uac_chip *uac = audio_dev->uac;
> @@ -630,6 +644,7 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
>  {
>  	struct snd_uac_chip *uac = audio_dev->uac;
>  
> +	stop_substream(&uac->c_prm);
>  	set_reported_srate(&uac->c_prm, 0);
>  	if (audio_dev->in_ep_fback)
>  		free_ep_fback(&uac->c_prm, audio_dev->in_ep_fback);
> @@ -713,6 +728,7 @@ void u_audio_stop_playback(struct g_audio *audio_dev)
>  {
>  	struct snd_uac_chip *uac = audio_dev->uac;
>  
> +	stop_substream(&uac->p_prm);
>  	set_reported_srate(&uac->p_prm, 0);
>  	free_ep(&uac->p_prm, audio_dev->in_ep);
>  }
> -- 
> 2.25.1
>
Pavel Hofman Dec. 22, 2021, 12:26 p.m. UTC | #2
Dne 21. 12. 21 v 13:18 John Keeping napsal(a):
> On Mon, Dec 20, 2021 at 10:11:26PM +0100, Pavel Hofman wrote:
>> When the USB host stops capture/playback, the corresponding PCM
>> substream (if activated and running) is stopped and drained.
>>
>> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
>> ---
>>   drivers/usb/gadget/function/u_audio.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
>> index a6293415c071..9dbce51c2eb7 100644
>> --- a/drivers/usb/gadget/function/u_audio.c
>> +++ b/drivers/usb/gadget/function/u_audio.c
>> @@ -544,6 +544,20 @@ static void set_reported_srate(struct uac_rtd_params *prm, int srate)
>>   	}
>>   }
>>   
>> +static void stop_substream(struct uac_rtd_params *prm)
>> +{
>> +	unsigned long _flags;
>> +	struct snd_pcm_substream *substream;
>> +
>> +	substream = prm->ss;
>> +	if (substream) {
>> +		snd_pcm_stream_lock_irqsave(substream, _flags);
>> +		if (snd_pcm_running(substream))
>> +			snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
> 
> I'm not sure if this is right (and the series should probably be CC'd to
> alsa-devel to check the audio side of this).
> 
> DRAINING seems to be right for capture, but for playback should this end
> up in state SETUP?  Does this need to handle resuming a paused stream
> like snd_pcm_drain() does?

Honestly, I do not know. This code comes from a SPDIF receiver driver 
where it handles interrupted incoming SPDIF stream. You are right it is 
related to capture. I will ask the alsa devs about the playback solution 
specifically.

Yes I will CC the next version to alsa-dev just in case.

> 
>> +		snd_pcm_stream_unlock_irqrestore(substream, _flags);
>> +	}
>> +}
>> +
>>   int u_audio_start_capture(struct g_audio *audio_dev)
>>   {
>>   	struct snd_uac_chip *uac = audio_dev->uac;
>> @@ -630,6 +644,7 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
>>   {
>>   	struct snd_uac_chip *uac = audio_dev->uac;
>>   
>> +	stop_substream(&uac->c_prm);
>>   	set_reported_srate(&uac->c_prm, 0);
>>   	if (audio_dev->in_ep_fback)
>>   		free_ep_fback(&uac->c_prm, audio_dev->in_ep_fback);
>> @@ -713,6 +728,7 @@ void u_audio_stop_playback(struct g_audio *audio_dev)
>>   {
>>   	struct snd_uac_chip *uac = audio_dev->uac;
>>   
>> +	stop_substream(&uac->p_prm);
>>   	set_reported_srate(&uac->p_prm, 0);
>>   	free_ep(&uac->p_prm, audio_dev->in_ep);
>>   }
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index a6293415c071..9dbce51c2eb7 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -544,6 +544,20 @@  static void set_reported_srate(struct uac_rtd_params *prm, int srate)
 	}
 }
 
+static void stop_substream(struct uac_rtd_params *prm)
+{
+	unsigned long _flags;
+	struct snd_pcm_substream *substream;
+
+	substream = prm->ss;
+	if (substream) {
+		snd_pcm_stream_lock_irqsave(substream, _flags);
+		if (snd_pcm_running(substream))
+			snd_pcm_stop(substream, SNDRV_PCM_STATE_DRAINING);
+		snd_pcm_stream_unlock_irqrestore(substream, _flags);
+	}
+}
+
 int u_audio_start_capture(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
@@ -630,6 +644,7 @@  void u_audio_stop_capture(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
 
+	stop_substream(&uac->c_prm);
 	set_reported_srate(&uac->c_prm, 0);
 	if (audio_dev->in_ep_fback)
 		free_ep_fback(&uac->c_prm, audio_dev->in_ep_fback);
@@ -713,6 +728,7 @@  void u_audio_stop_playback(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
 
+	stop_substream(&uac->p_prm);
 	set_reported_srate(&uac->p_prm, 0);
 	free_ep(&uac->p_prm, audio_dev->in_ep);
 }