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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F08CC433E0 for ; Wed, 3 Jun 2020 06:28:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3007820679 for ; Wed, 3 Jun 2020 06:28:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725995AbgFCG2P (ORCPT ); Wed, 3 Jun 2020 02:28:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:57152 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725810AbgFCG2O (ORCPT ); Wed, 3 Jun 2020 02:28:14 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C4442AD10; Wed, 3 Jun 2020 06:28:13 +0000 (UTC) Date: Wed, 03 Jun 2020 08:28:09 +0200 Message-ID: From: Takashi Iwai To: Macpaul Lin Cc: Jaroslav Kysela , Takashi Iwai , Matthias Brugger , Alexander Tsoy , Johan Hovold , Hui Wang , Szabolcs =?UTF-8?B?U3rFkWtl?= , , , Mediatek WSD Upstream , Macpaul Lin , , , , Greg Kroah-Hartman Subject: Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend In-Reply-To: <1591153515.23525.50.camel@mtkswgap22> References: <1591153515.23525.50.camel@mtkswgap22> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 03 Jun 2020 05:05:15 +0200, Macpaul Lin wrote: > > On Tue, 2020-06-02 at 14:46 +0200, Takashi Iwai wrote: > > On Tue, 02 Jun 2020 13:53:41 +0200, > > Macpaul Lin wrote: > > > > > > This patch fix incorrect power state changed by usb_audio_suspend() > > > when CONFIG_PM is enabled. > > > > > > After receiving suspend PM message with auto flag, usb_audio_suspend() > > > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > > > resume PM message with auto flag can change power state to > > > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > > > > > However, when system is not under auto suspend, resume PM message with > > > auto flag might not be able to receive on time which cause the power > > > state was incorrect. At this time, if a player starts to play sound, > > > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > > > resume the card. > > > > > > But even the card is back to work and all function normal, the power > > > state is still in SNDRV_CTL_POWER_D3hot. > > > > Hm, in exactly which situation does this happen? I still don't get > > it. Could you elaborate how to trigger this? > > I'm not sure if this will happen on laptop or on PC. > We've found this issue on Android phone (I'm not sure if each Android > phone can reproduce this.). > > After booting the android phone, insert type-c headset without charging > and play music at any duration, say, 1 second, then stop. Put phone away > to idle about 17~18 minutes. Wait auto pm happened and the power state > change to SNDRV_CTL_POWER_D3hot in sound/usb/card.c. Then wake up the > phone, play music again. Then you'll probably found the music was not > playing and the progress bar keep at the same position. It only happen > when power state is SNDRV_CTL_POWER_D3hot. If not (the power state is > SNDRV_CTL_POWER_D0), repeat the steps for several times, then it will be > produced at some time. > > When it happened, sound_usb_pcm_open() will wake up the sound card by > setup_hw_info()->__usb_audio_resume(). However, the card and the > interface is function properly right now, the power state keeps remain > SNDRV_CTL_POWER_D3hot. And at this point it's already something wrong. We need to check why SNDRV_CTL_POWER_D3hot is kept there, instead of working around the rest behavior. > The suggestive parameter settings from upper > sound request will be pending since later snd_power_wait() call will > still wait the card awaken. Ideally, auto PM should be recovered by > sound card itself. But once the card is awaken at this circumstance, it > looks like there are not more auto pm event. And the sound system of > this interface will stuck here forever until user plug out the headset > (reset the hardware). > > The root cause is that once the card has been resumed, it should inform > auto pm change the state back into SNDRV_CTL_POWER_D0 and mark the > device is using by some one. > > > > Which cause the infinite loop > > > happened in snd_power_wait() to check the power state. Thus the > > > successive setting ioctl cannot be passed to card. > > > > > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > > > has been resumed successfully. > > > > This doesn't look like a right solution for the problem, sorry. > > The card PM status must be recovered to D0 when the autoresume > > succeeds. If not, something is broken there, and it must be fixed > > instead of fiddling the status flag externally. > > Yes, I agreed, but after checking the code in sound drivers, > it looks like there is only chance that auto pm triggered by low-level > code in sound/usb/card.c. In kernel 4.14, auto pm suspend is triggered > by snd_pcm_suspend_all(). In later kernel, it is triggered by > snd_usb_pcm_suspend(). However, it looks like there are no any resume > trigger to recover auto pm state when the card has been waken by > sound_usb_pcm_open(). If a running PCM stream has been suspended, the stream needs to be resumed manually by user-space. There is no automatic resume. You can forget about it and skip scratching that surface. Again, the point to be checked is why D3hot is kept after snd_usb_autoresume() is called. It's Android, and I wonder whether the system does the system-suspend (S3), or it's all runtime PM? Basically D3hot is set only for the former, the system suspend, where the driver's PM callback is called with PMSG_SUSPEND. Please check this at first. That is, usb_audio_suspend() receives PMSG_SUSPEND or such, which makes chip->autosuspended=1. The D3hot flag is set only in this condition. Then, check the resume patterns. The usb-audio suspend/resume has multiple refcounts. One is the Linux device PM refcount, and chip->active refcount, and chip->num_suspended_intf refcount. The first one (PM refount) is the primary refcount to manage the whole system, and this is incremented / decremented by the standard PM calls. The second one, chip->active, is a temporary flag to avoid the re-entrance of the PM callbacks, and incremented at the probe enter and __usb_audio_resume(), and decremented at the probe exit and __usb_audio_resume() exist. The last one, chip->num_suspended_intf is a refcount for the multiple interfaces assigned to a single card. And, the most suspicious case is the last one, chip->num_suspended-intf. It means that the device has multiple USB interfaces and they went to suspend, while the resume isn't performed for the all suspended interfaces in return. If that's the case, you need to check where the suspend gets called to which USB-interface (and which pm_message_t) and whether the resume gets called for those. thanks, Takashi 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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE646C433E0 for ; Wed, 3 Jun 2020 06:29:16 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 79BF120679 for ; Wed, 3 Jun 2020 06:29:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="n2kI/UHi" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 79BF120679 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id E53531614; Wed, 3 Jun 2020 08:28:24 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz E53531614 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1591165755; bh=zP0WjWeNMfh0CCGDUxu9wRgO5CDZ6Mgz3AbNEMyCtaM=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=n2kI/UHizN0NMsxvL1lu3oxkr6v/PS/9cbW8804UY4z9izY5uXsTF+5xQEBCSTAQF jRMOn3eH27lXVKEhX4O7YkXUkgMQgwmFLXzOn9WzIA4tWIWoTKiX+oJrGpcZpf3M/s qZcTvjLVrQQq39pGyyb4K0muXGNPP5PuWxX/lmas= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 6108BF801EC; Wed, 3 Jun 2020 08:28:24 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id B4245F801ED; Wed, 3 Jun 2020 08:28:19 +0200 (CEST) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id BF010F800BC for ; Wed, 3 Jun 2020 08:28:13 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz BF010F800BC X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C4442AD10; Wed, 3 Jun 2020 06:28:13 +0000 (UTC) Date: Wed, 03 Jun 2020 08:28:09 +0200 Message-ID: From: Takashi Iwai To: Macpaul Lin Subject: Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend In-Reply-To: <1591153515.23525.50.camel@mtkswgap22> References: <1591153515.23525.50.camel@mtkswgap22> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Cc: alsa-devel@alsa-project.org, Mediatek WSD Upstream , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Johan Hovold , Takashi Iwai , Hui Wang , Alexander Tsoy , linux-mediatek@lists.infradead.org, Greg Kroah-Hartman , Matthias Brugger , Macpaul Lin , Szabolcs =?UTF-8?B?U3rFkWtl?= , linux-arm-kernel@lists.infradead.org X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Wed, 03 Jun 2020 05:05:15 +0200, Macpaul Lin wrote: > > On Tue, 2020-06-02 at 14:46 +0200, Takashi Iwai wrote: > > On Tue, 02 Jun 2020 13:53:41 +0200, > > Macpaul Lin wrote: > > > > > > This patch fix incorrect power state changed by usb_audio_suspend() > > > when CONFIG_PM is enabled. > > > > > > After receiving suspend PM message with auto flag, usb_audio_suspend() > > > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > > > resume PM message with auto flag can change power state to > > > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > > > > > However, when system is not under auto suspend, resume PM message with > > > auto flag might not be able to receive on time which cause the power > > > state was incorrect. At this time, if a player starts to play sound, > > > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > > > resume the card. > > > > > > But even the card is back to work and all function normal, the power > > > state is still in SNDRV_CTL_POWER_D3hot. > > > > Hm, in exactly which situation does this happen? I still don't get > > it. Could you elaborate how to trigger this? > > I'm not sure if this will happen on laptop or on PC. > We've found this issue on Android phone (I'm not sure if each Android > phone can reproduce this.). > > After booting the android phone, insert type-c headset without charging > and play music at any duration, say, 1 second, then stop. Put phone away > to idle about 17~18 minutes. Wait auto pm happened and the power state > change to SNDRV_CTL_POWER_D3hot in sound/usb/card.c. Then wake up the > phone, play music again. Then you'll probably found the music was not > playing and the progress bar keep at the same position. It only happen > when power state is SNDRV_CTL_POWER_D3hot. If not (the power state is > SNDRV_CTL_POWER_D0), repeat the steps for several times, then it will be > produced at some time. > > When it happened, sound_usb_pcm_open() will wake up the sound card by > setup_hw_info()->__usb_audio_resume(). However, the card and the > interface is function properly right now, the power state keeps remain > SNDRV_CTL_POWER_D3hot. And at this point it's already something wrong. We need to check why SNDRV_CTL_POWER_D3hot is kept there, instead of working around the rest behavior. > The suggestive parameter settings from upper > sound request will be pending since later snd_power_wait() call will > still wait the card awaken. Ideally, auto PM should be recovered by > sound card itself. But once the card is awaken at this circumstance, it > looks like there are not more auto pm event. And the sound system of > this interface will stuck here forever until user plug out the headset > (reset the hardware). > > The root cause is that once the card has been resumed, it should inform > auto pm change the state back into SNDRV_CTL_POWER_D0 and mark the > device is using by some one. > > > > Which cause the infinite loop > > > happened in snd_power_wait() to check the power state. Thus the > > > successive setting ioctl cannot be passed to card. > > > > > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > > > has been resumed successfully. > > > > This doesn't look like a right solution for the problem, sorry. > > The card PM status must be recovered to D0 when the autoresume > > succeeds. If not, something is broken there, and it must be fixed > > instead of fiddling the status flag externally. > > Yes, I agreed, but after checking the code in sound drivers, > it looks like there is only chance that auto pm triggered by low-level > code in sound/usb/card.c. In kernel 4.14, auto pm suspend is triggered > by snd_pcm_suspend_all(). In later kernel, it is triggered by > snd_usb_pcm_suspend(). However, it looks like there are no any resume > trigger to recover auto pm state when the card has been waken by > sound_usb_pcm_open(). If a running PCM stream has been suspended, the stream needs to be resumed manually by user-space. There is no automatic resume. You can forget about it and skip scratching that surface. Again, the point to be checked is why D3hot is kept after snd_usb_autoresume() is called. It's Android, and I wonder whether the system does the system-suspend (S3), or it's all runtime PM? Basically D3hot is set only for the former, the system suspend, where the driver's PM callback is called with PMSG_SUSPEND. Please check this at first. That is, usb_audio_suspend() receives PMSG_SUSPEND or such, which makes chip->autosuspended=1. The D3hot flag is set only in this condition. Then, check the resume patterns. The usb-audio suspend/resume has multiple refcounts. One is the Linux device PM refcount, and chip->active refcount, and chip->num_suspended_intf refcount. The first one (PM refount) is the primary refcount to manage the whole system, and this is incremented / decremented by the standard PM calls. The second one, chip->active, is a temporary flag to avoid the re-entrance of the PM callbacks, and incremented at the probe enter and __usb_audio_resume(), and decremented at the probe exit and __usb_audio_resume() exist. The last one, chip->num_suspended_intf is a refcount for the multiple interfaces assigned to a single card. And, the most suspicious case is the last one, chip->num_suspended-intf. It means that the device has multiple USB interfaces and they went to suspend, while the resume isn't performed for the all suspended interfaces in return. If that's the case, you need to check where the suspend gets called to which USB-interface (and which pm_message_t) and whether the resume gets called for those. thanks, Takashi 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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8DC87C433E0 for ; Wed, 3 Jun 2020 06:28:35 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 60F6A20679 for ; Wed, 3 Jun 2020 06:28:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="MK+utPcP" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 60F6A20679 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BYAjz+c4znwBxBEZ03Hp/It0M7utCGxPWQFh2NxtV4E=; b=MK+utPcPBQw8uo orZjev4UGnRTPbau20IGI3B6vCU+qM+jvElT1WSApChJgaS1Ds+dWzQplHUMJf58qgufRBH67AXFw xwKijaxv82rytWL9ChRvyI9T3tVnQ0TZM7Fd2gCrpO9JIedlOXo59wfkz22aRdMURcQKuWf8G38uL RSx3NH5hUo6W+Yt51Jt1ENdHTZs1iSqIdPGTVqELkOSpmavO88ecpEtebJqU0cdzs/qdFbZmcEtmZ SmJjC1Zh2GGOhyHwv2/aNEVBigy2hOqTZxDCkAeXk41h4XwfthRaLrbje7NtjMnZgncVJI1X1SdPf WznMzPjkghDPPJo0BSvw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgMsz-00013G-Kt; Wed, 03 Jun 2020 06:28:25 +0000 Received: from mx2.suse.de ([195.135.220.15]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgMso-0000vI-Iv; Wed, 03 Jun 2020 06:28:18 +0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C4442AD10; Wed, 3 Jun 2020 06:28:13 +0000 (UTC) Date: Wed, 03 Jun 2020 08:28:09 +0200 Message-ID: From: Takashi Iwai To: Macpaul Lin Subject: Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend In-Reply-To: <1591153515.23525.50.camel@mtkswgap22> References: <1591153515.23525.50.camel@mtkswgap22> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200602_232814_916679_FDE69D54 X-CRM114-Status: GOOD ( 32.73 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, Mediatek WSD Upstream , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Johan Hovold , Takashi Iwai , Hui Wang , Alexander Tsoy , linux-mediatek@lists.infradead.org, Greg Kroah-Hartman , Matthias Brugger , Macpaul Lin , Jaroslav Kysela , Szabolcs =?UTF-8?B?U3rFkWtl?= , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Wed, 03 Jun 2020 05:05:15 +0200, Macpaul Lin wrote: > > On Tue, 2020-06-02 at 14:46 +0200, Takashi Iwai wrote: > > On Tue, 02 Jun 2020 13:53:41 +0200, > > Macpaul Lin wrote: > > > > > > This patch fix incorrect power state changed by usb_audio_suspend() > > > when CONFIG_PM is enabled. > > > > > > After receiving suspend PM message with auto flag, usb_audio_suspend() > > > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > > > resume PM message with auto flag can change power state to > > > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > > > > > However, when system is not under auto suspend, resume PM message with > > > auto flag might not be able to receive on time which cause the power > > > state was incorrect. At this time, if a player starts to play sound, > > > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > > > resume the card. > > > > > > But even the card is back to work and all function normal, the power > > > state is still in SNDRV_CTL_POWER_D3hot. > > > > Hm, in exactly which situation does this happen? I still don't get > > it. Could you elaborate how to trigger this? > > I'm not sure if this will happen on laptop or on PC. > We've found this issue on Android phone (I'm not sure if each Android > phone can reproduce this.). > > After booting the android phone, insert type-c headset without charging > and play music at any duration, say, 1 second, then stop. Put phone away > to idle about 17~18 minutes. Wait auto pm happened and the power state > change to SNDRV_CTL_POWER_D3hot in sound/usb/card.c. Then wake up the > phone, play music again. Then you'll probably found the music was not > playing and the progress bar keep at the same position. It only happen > when power state is SNDRV_CTL_POWER_D3hot. If not (the power state is > SNDRV_CTL_POWER_D0), repeat the steps for several times, then it will be > produced at some time. > > When it happened, sound_usb_pcm_open() will wake up the sound card by > setup_hw_info()->__usb_audio_resume(). However, the card and the > interface is function properly right now, the power state keeps remain > SNDRV_CTL_POWER_D3hot. And at this point it's already something wrong. We need to check why SNDRV_CTL_POWER_D3hot is kept there, instead of working around the rest behavior. > The suggestive parameter settings from upper > sound request will be pending since later snd_power_wait() call will > still wait the card awaken. Ideally, auto PM should be recovered by > sound card itself. But once the card is awaken at this circumstance, it > looks like there are not more auto pm event. And the sound system of > this interface will stuck here forever until user plug out the headset > (reset the hardware). > > The root cause is that once the card has been resumed, it should inform > auto pm change the state back into SNDRV_CTL_POWER_D0 and mark the > device is using by some one. > > > > Which cause the infinite loop > > > happened in snd_power_wait() to check the power state. Thus the > > > successive setting ioctl cannot be passed to card. > > > > > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > > > has been resumed successfully. > > > > This doesn't look like a right solution for the problem, sorry. > > The card PM status must be recovered to D0 when the autoresume > > succeeds. If not, something is broken there, and it must be fixed > > instead of fiddling the status flag externally. > > Yes, I agreed, but after checking the code in sound drivers, > it looks like there is only chance that auto pm triggered by low-level > code in sound/usb/card.c. In kernel 4.14, auto pm suspend is triggered > by snd_pcm_suspend_all(). In later kernel, it is triggered by > snd_usb_pcm_suspend(). However, it looks like there are no any resume > trigger to recover auto pm state when the card has been waken by > sound_usb_pcm_open(). If a running PCM stream has been suspended, the stream needs to be resumed manually by user-space. There is no automatic resume. You can forget about it and skip scratching that surface. Again, the point to be checked is why D3hot is kept after snd_usb_autoresume() is called. It's Android, and I wonder whether the system does the system-suspend (S3), or it's all runtime PM? Basically D3hot is set only for the former, the system suspend, where the driver's PM callback is called with PMSG_SUSPEND. Please check this at first. That is, usb_audio_suspend() receives PMSG_SUSPEND or such, which makes chip->autosuspended=1. The D3hot flag is set only in this condition. Then, check the resume patterns. The usb-audio suspend/resume has multiple refcounts. One is the Linux device PM refcount, and chip->active refcount, and chip->num_suspended_intf refcount. The first one (PM refount) is the primary refcount to manage the whole system, and this is incremented / decremented by the standard PM calls. The second one, chip->active, is a temporary flag to avoid the re-entrance of the PM callbacks, and incremented at the probe enter and __usb_audio_resume(), and decremented at the probe exit and __usb_audio_resume() exist. The last one, chip->num_suspended_intf is a refcount for the multiple interfaces assigned to a single card. And, the most suspicious case is the last one, chip->num_suspended-intf. It means that the device has multiple USB interfaces and they went to suspend, while the resume isn't performed for the all suspended interfaces in return. If that's the case, you need to check where the suspend gets called to which USB-interface (and which pm_message_t) and whether the resume gets called for those. thanks, Takashi _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2E48EC433E1 for ; Wed, 3 Jun 2020 06:28:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0498F20679 for ; Wed, 3 Jun 2020 06:28:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="de0u2CB2" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0498F20679 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=yGkeTj0kxUNPvpWxPo8AGkQrNkGujW5mIpyH5o9dmzM=; b=de0u2CB2wMNt3C zqKYyjnKrug4Dnxv8IyeTGHAoXWWwiiw3eWiLC7JBLpGolr//cXfffpTkShxlUvOMIRJZ3syQ9TF6 FlJcANDTs6/Zn1B0InBJtxNwvyLbFgA7S4MS0q+2rHedetryyM3y6C15X33dVrvUNLQXpnv/LEWzt d92xDOuqNQLU9nC0mRHm4pVxY20LmpaMez4xKj9iHC2xWoyenRhw9HWvs6eFXOn0V7LYdY0/UQlKm Y/mHIBfZFTQ9L7S/E/5p3xbRvnkCxRTTgOddQT4xnAMM2/z/t75wtWsYBhfhSO0Syso2hat5qX5Cm oKuFJg95xG7IpbmrWj9w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgMsu-0000vw-Io; Wed, 03 Jun 2020 06:28:20 +0000 Received: from mx2.suse.de ([195.135.220.15]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jgMso-0000vI-Iv; Wed, 03 Jun 2020 06:28:18 +0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C4442AD10; Wed, 3 Jun 2020 06:28:13 +0000 (UTC) Date: Wed, 03 Jun 2020 08:28:09 +0200 Message-ID: From: Takashi Iwai To: Macpaul Lin Subject: Re: [PATCH] sound: usb: pcm: fix incorrect power state when playing sound after PM_AUTO suspend In-Reply-To: <1591153515.23525.50.camel@mtkswgap22> References: <1591153515.23525.50.camel@mtkswgap22> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200602_232814_916679_FDE69D54 X-CRM114-Status: GOOD ( 32.73 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, Mediatek WSD Upstream , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, Johan Hovold , Takashi Iwai , Hui Wang , Alexander Tsoy , linux-mediatek@lists.infradead.org, Greg Kroah-Hartman , Matthias Brugger , Macpaul Lin , Jaroslav Kysela , Szabolcs =?UTF-8?B?U3rFkWtl?= , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 03 Jun 2020 05:05:15 +0200, Macpaul Lin wrote: > > On Tue, 2020-06-02 at 14:46 +0200, Takashi Iwai wrote: > > On Tue, 02 Jun 2020 13:53:41 +0200, > > Macpaul Lin wrote: > > > > > > This patch fix incorrect power state changed by usb_audio_suspend() > > > when CONFIG_PM is enabled. > > > > > > After receiving suspend PM message with auto flag, usb_audio_suspend() > > > change card's power state to SNDRV_CTL_POWER_D3hot. Only when the other > > > resume PM message with auto flag can change power state to > > > SNDRV_CTL_POWER_D0 in __usb_audio_resume(). > > > > > > However, when system is not under auto suspend, resume PM message with > > > auto flag might not be able to receive on time which cause the power > > > state was incorrect. At this time, if a player starts to play sound, > > > will cause snd_usb_pcm_open() to access the card and setup_hw_info() will > > > resume the card. > > > > > > But even the card is back to work and all function normal, the power > > > state is still in SNDRV_CTL_POWER_D3hot. > > > > Hm, in exactly which situation does this happen? I still don't get > > it. Could you elaborate how to trigger this? > > I'm not sure if this will happen on laptop or on PC. > We've found this issue on Android phone (I'm not sure if each Android > phone can reproduce this.). > > After booting the android phone, insert type-c headset without charging > and play music at any duration, say, 1 second, then stop. Put phone away > to idle about 17~18 minutes. Wait auto pm happened and the power state > change to SNDRV_CTL_POWER_D3hot in sound/usb/card.c. Then wake up the > phone, play music again. Then you'll probably found the music was not > playing and the progress bar keep at the same position. It only happen > when power state is SNDRV_CTL_POWER_D3hot. If not (the power state is > SNDRV_CTL_POWER_D0), repeat the steps for several times, then it will be > produced at some time. > > When it happened, sound_usb_pcm_open() will wake up the sound card by > setup_hw_info()->__usb_audio_resume(). However, the card and the > interface is function properly right now, the power state keeps remain > SNDRV_CTL_POWER_D3hot. And at this point it's already something wrong. We need to check why SNDRV_CTL_POWER_D3hot is kept there, instead of working around the rest behavior. > The suggestive parameter settings from upper > sound request will be pending since later snd_power_wait() call will > still wait the card awaken. Ideally, auto PM should be recovered by > sound card itself. But once the card is awaken at this circumstance, it > looks like there are not more auto pm event. And the sound system of > this interface will stuck here forever until user plug out the headset > (reset the hardware). > > The root cause is that once the card has been resumed, it should inform > auto pm change the state back into SNDRV_CTL_POWER_D0 and mark the > device is using by some one. > > > > Which cause the infinite loop > > > happened in snd_power_wait() to check the power state. Thus the > > > successive setting ioctl cannot be passed to card. > > > > > > Hence we suggest to change power state to SNDRV_CTL_POWER_D0 when card > > > has been resumed successfully. > > > > This doesn't look like a right solution for the problem, sorry. > > The card PM status must be recovered to D0 when the autoresume > > succeeds. If not, something is broken there, and it must be fixed > > instead of fiddling the status flag externally. > > Yes, I agreed, but after checking the code in sound drivers, > it looks like there is only chance that auto pm triggered by low-level > code in sound/usb/card.c. In kernel 4.14, auto pm suspend is triggered > by snd_pcm_suspend_all(). In later kernel, it is triggered by > snd_usb_pcm_suspend(). However, it looks like there are no any resume > trigger to recover auto pm state when the card has been waken by > sound_usb_pcm_open(). If a running PCM stream has been suspended, the stream needs to be resumed manually by user-space. There is no automatic resume. You can forget about it and skip scratching that surface. Again, the point to be checked is why D3hot is kept after snd_usb_autoresume() is called. It's Android, and I wonder whether the system does the system-suspend (S3), or it's all runtime PM? Basically D3hot is set only for the former, the system suspend, where the driver's PM callback is called with PMSG_SUSPEND. Please check this at first. That is, usb_audio_suspend() receives PMSG_SUSPEND or such, which makes chip->autosuspended=1. The D3hot flag is set only in this condition. Then, check the resume patterns. The usb-audio suspend/resume has multiple refcounts. One is the Linux device PM refcount, and chip->active refcount, and chip->num_suspended_intf refcount. The first one (PM refount) is the primary refcount to manage the whole system, and this is incremented / decremented by the standard PM calls. The second one, chip->active, is a temporary flag to avoid the re-entrance of the PM callbacks, and incremented at the probe enter and __usb_audio_resume(), and decremented at the probe exit and __usb_audio_resume() exist. The last one, chip->num_suspended_intf is a refcount for the multiple interfaces assigned to a single card. And, the most suspicious case is the last one, chip->num_suspended-intf. It means that the device has multiple USB interfaces and they went to suspend, while the resume isn't performed for the all suspended interfaces in return. If that's the case, you need to check where the suspend gets called to which USB-interface (and which pm_message_t) and whether the resume gets called for those. thanks, Takashi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel