Re: [PATCH] Add duplex sound support for USB devices using implicit feedback

[Erwin Burema <e.burema@freedom.nl>]

On zondag 24 mei 2020 10:37:34 CEST Takashi Iwai wrote:
> On Sat, 23 May 2020 20:09:31 +0200,
> 
> Erwin Burema wrote:
> > On zaterdag 23 mei 2020 19:53:49 CEST Alexander Tsoy wrote:
> > > В Вс, 10/05/2020 в 20:29 +0200, Erwin Burema пишет:
> > > > For USB sound devices using implicit feedback the endpoint used for
> > > > this feedback should be able to be opened twice, once for required
> > > > feedback and second time for audio data. This way these devices can
> > > > be put in duplex audio mode. Since this only works if the settings of
> > > > the endpoint don't change a check is included for this.
> > > > 
> > > > This fixes bug 207023 ("MOTU M2 regression on duplex audio") and
> > > > should also fix bug 103751 ("M-Audio Fast Track Ultra usb audio
> > > > device will not operate full-duplex")
> > > > 
> > > > Signed-off-by: Erwin Burema <e.burema@gmail.com>
> > > > ---
> > > 
> > > This patch seems to cause kernel panic on my system. This happens
> > > during boot when gdm (with pulseaudio) is starting up.
> > 
> > That's interesting, not running gnome (and thus also not running gdm,
> > currently KDE with SDDM) here so would need to take some time
> > troubleshooting. Suspect I missed something in the check if both input
> > and output are similarly configured.
> > 
> > Will see if I can reproduce it and where exactly it goes wrong, in the
> > meantime would it be possible to test if 5.6 or later show the same issue
> > since I intially developed the patch against that release?
> 
> Judging from the point triggering the bug (memset()), this can be a
> leftover URB handling that is performed even after the capture stream
> is closed.  If so, the procedure would be:
>  - start capture
>  - start playback
>  - stop capture while keeping playback running
> 
> If my guess is correct, can the patch below work around the issue?
> 

Have spend a large part of my free time trying to replicate it without any 
luck, might be due to tryin it in a VM with USB passtrough (but wanted to be 
able to quickly itterate and easier to get debug info), so haven't been able 
to try out the patch.

Next step is to see if I can replicate it on my HW, if that doesn't work it 
might be time to rethink this whole initial patch and might need to do 
something at substream level instead of endpoint level.

Regards,

Erwin Burema

> 
> thanks,
> 
> Takashi
> 
> ---
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1782,6 +1782,7 @@ static int snd_usb_substream_capture_trigger(struct
> snd_pcm_substream *substream return 0;
>  	case SNDRV_PCM_TRIGGER_STOP:
>  		stop_endpoints(subs);
> +		subs->data_endpoint->retire_data_urb = NULL;
>  		subs->running = 0;
>  		return 0;
>  	case SNDRV_PCM_TRIGGER_PAUSE_PUSH: