From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E36CDC4332F for ; Tue, 29 Nov 2022 07:14:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229533AbiK2HOF (ORCPT ); Tue, 29 Nov 2022 02:14:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52036 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229925AbiK2HNq (ORCPT ); Tue, 29 Nov 2022 02:13:46 -0500 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B48022AC71 for ; Mon, 28 Nov 2022 23:13:43 -0800 (PST) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 4AE201FDDE; Tue, 29 Nov 2022 07:13:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1669706022; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=QiMam+oshjx/yNuPlMI/ctZOd3E1xzj79VjC523hfhg=; b=ZGqZ0RZzvPAzwA0aYaGe9sZJrjavEpcHfsTSG6iURrYBxT5BPbUrh/zAUc7OZHFhOVdjzf zktlqcj44mJNJ72NYM6iztCz2B0SUVu1ErMAfHdwMTfgU84ei9AqISl47vFMUIqxEZpxpp wdewX7E8FoVLDDYMJH2tkYS4NwEW9dA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1669706022; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=QiMam+oshjx/yNuPlMI/ctZOd3E1xzj79VjC523hfhg=; b=Pp2lRUouRGIydu9M5Uw3wEPduO1qHZZ2bYvoSj+6JDuEGGjFYCySrbWL224ireRAFzp0bV wy7r7iaHpheLZ8DA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 176EB13428; Tue, 29 Nov 2022 07:13:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id nVbpBCaxhWMjXQAAMHmgww (envelope-from ); Tue, 29 Nov 2022 07:13:42 +0000 Date: Tue, 29 Nov 2022 08:13:41 +0100 Message-ID: <87h6yiqllm.wl-tiwai@suse.de> From: Takashi Iwai To: John Keeping Cc: alsa-devel@alsa-project.org, Jaroslav Kysela , Takashi Iwai , linux-kernel@vger.kernel.org Subject: Re: [PATCH] ALSA: usb-audio: Add quirk for Tascam Model 12 In-Reply-To: References: <20221128122353.763696-1-john@metanate.com> <87lenvyyp8.wl-tiwai@suse.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 28 Nov 2022 20:20:05 +0100, John Keeping wrote: > > On Mon, Nov 28, 2022 at 02:51:15PM +0100, Takashi Iwai wrote: > > On Mon, 28 Nov 2022 13:23:52 +0100, > > John Keeping wrote: > > > > > > Tascam's Model 12 is a mixer which can also operate as a USB audio > > > interface. The audio interface uses explicit feedback but it seems that > > > it does not correctly handle missing isochronous frames. > > > > > > When injecting an xrun (or doing anything else that pauses the playback > > > stream) the feedback rate climbs (for example, at 44,100Hz nominal, I > > > see a stable rate around 44,099 but xrun injection sees this peak at > > > around 44,135 in most cases) and glitches are heard in the audio stream > > > for several seconds - this is significantly worse than the single glitch > > > expected for an underrun. > > > > > > While the stream does normally recover and the feedback rate returns to > > > a stable value, I have seen some occurrences where this does not happen > > > and the rate continues to increase while no audio is heard from the > > > output. I have not found a solid reproduction for this. > > > > > > This misbehaviour can be avoided by totally resetting the stream state > > > by switching the interface to alt 0 and back before restarting the > > > playback stream. > > > > > > Add a new quirk flag which forces the endpoint and interface to be > > > reconfigured whenever the stream is stopped, and use this for the Tascam > > > Model 12. > > > > > > Signed-off-by: John Keeping > > > > Thanks for the patch, it's an interesting case. > > About the code change: > > > > > --- a/sound/usb/endpoint.c > > > +++ b/sound/usb/endpoint.c > > > @@ -1673,6 +1673,13 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending) > > > stop_urbs(ep, false, keep_pending); > > > if (ep->clock_ref) > > > atomic_dec(&ep->clock_ref->locked); > > > + > > > + if (ep->chip->quirk_flags & QUIRK_FLAG_FORCE_IFACE_RESET && > > > + usb_pipeout(ep->pipe)) { > > > + ep->need_setup = true; > > It seems I missed this when forward porting the patch from 5.15 - this > should be setting ep->need_prepare and will change in v2. > > > > + if (ep->iface_ref) > > > + ep->iface_ref->need_setup = true; > > > + } > > > > Is this the forced reset always safe? Imagine that you have > > individual playback and capture streams, and what if only one of them > > gets stopped and restarted while another keeps running? > > I /think/ this is okay because the interfaces for capture & playback are > separate (although the clock is shared). > > There are two endpoints on the playback interface - the playback data > and explicit feedback endpoints - but these are always stopped and > started at the same time so I can't see any problem here. (Only the > data endpoint will trigger the reset request here due to the > usb_pipeout() check.) Ah OK, then it should be safe -- and it'd be worth to mention it in the changelog, too (hint for the resubmission :) Takashi