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=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham 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 508CDC432BE for ; Thu, 26 Aug 2021 15:30:38 +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 C03BB60E0B for ; Thu, 26 Aug 2021 15:30:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C03BB60E0B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=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 E302C16CC; Thu, 26 Aug 2021 17:29:45 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz E302C16CC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1629991836; bh=90FfHWMEqGTB9nWjgafwfJKZByff1bO1JDl1HdyurBM=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=eDkSjRFAw9QYuftwM3Kof9GiHb/pu8la2FzpGyty7KuUoDlSc1OrsArbv9V/gvc5Z oJaSjdkW6aNvVLZdk9i7xUcGMiwgGTWy1IvAkI+M4MaM67UO3e+z6i7IgxndJni0u/ +8NmLH+ltGBCSo0YrJmS7Z22ZKvUvHNUVjx0/0Pc= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 28552F801D5; Thu, 26 Aug 2021 17:29:44 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 7FB60F8032D; Thu, 26 Aug 2021 17:29:29 +0200 (CEST) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D469FF801D5 for ; Thu, 26 Aug 2021 17:29:25 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D469FF801D5 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="a2Ww2tWF"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="vT6cTfTX" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 274102233F; Thu, 26 Aug 2021 15:29:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1629991765; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dRQF/+tjSycoJA7su7/7jCokD7Qrp9zjSDvhc+damsQ=; b=a2Ww2tWF8xTJk05cgA6hw8GpvvZ/uqzJlKC3VzhVRKsTaoGRWvPsr3CkkTQOwuBKrHXrKB tp1MVLSkhx+ZrNjEImd2lnKviw0dViOK4PM6zGStV2sMTw3QZmICbrEE48KeRLMfaZs8Vd XXNC+Z1kVvbTiS5Kwm322YF10qkHx2o= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1629991765; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dRQF/+tjSycoJA7su7/7jCokD7Qrp9zjSDvhc+damsQ=; b=vT6cTfTXESzSaSroLQ2uvdx453oPygdwTZ1ktJ4Z9vWaTqLBVAYrSxjsz9XHt7ZHaZtI1G MY7Mj0CVj8hKHFBA== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 213FBA3B8F; Thu, 26 Aug 2021 15:29:25 +0000 (UTC) Date: Thu, 26 Aug 2021 17:29:25 +0200 Message-ID: From: Takashi Iwai To: Vitaly Rodionov Subject: Re: [PATCH 2/2] ALSA: hda/cs8409: Prevent pops and clicks during reboot In-Reply-To: <3c2cee1d-8147-9521-99ba-6a8ade422529@opensource.cirrus.com> References: <20210812183433.6330-1-vitalyr@opensource.cirrus.com> <20210812183433.6330-2-vitalyr@opensource.cirrus.com> <6595e87d-1dae-b536-c17b-eafa07d04bbe@opensource.cirrus.com> <9a3c2f9e-e2a5-702f-bd3f-7348097a0500@opensource.cirrus.com> <3c2cee1d-8147-9521-99ba-6a8ade422529@opensource.cirrus.com> 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=UTF-8 Content-Transfer-Encoding: 8bit Cc: alsa-devel@alsa-project.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 Thu, 26 Aug 2021 17:14:31 +0200, Vitaly Rodionov wrote: > > On 26/08/2021 2:00 pm, Vitaly Rodionov wrote: > > On 26/08/2021 1:20 pm, Takashi Iwai wrote: > >> On Thu, 26 Aug 2021 13:49:32 +0200, > >> Vitaly Rodionov wrote: > >>> On 26/08/2021 11:49 am, Takashi Iwai wrote: > >>>> On Thu, 26 Aug 2021 08:03:45 +0200, > >>>> Takashi Iwai wrote: > >>>>> On Wed, 25 Aug 2021 20:04:05 +0200, > >>>>> Vitaly Rodionov wrote: > >>>>>> Actually when codec is suspended and we do reboot from UI, then > >>>>>> sometimes we > >>>>>> see suspend() calls in kernel log and no pops, but sometimes > >>>>>> > >>>>>> we still have no suspend() on reboot and we hear pops. But when > >>>>>> we do reboot > >>>>>> from command line: > sudo reboot  then we always have pops and > >>>>>> no suspend() > >>>>>> called. > >>>>>> > >>>>>> Then we have added extra logging and we can see that on reboot > >>>>>> codec somehow > >>>>>> getting resume() call and we run jack detect on resume that > >>>>>> causing pops. > >>>>> Hm, it's interesting who triggers the runtime resume. > >>>>> > >>>>>> We were thinking about possible solution for that and we would > >>>>>> propose some > >>>>>> changes in generic code hda_bind.c: > >>>>>> > >>>>>> static void hda_codec_driver_shutdown(struct device *dev) { +   > >>>>>> if (codec-> > >>>>>> patch_ops.suspend) + codec->patch_ops.suspend(codec); > >>>>>> snd_hda_codec_shutdown(dev_to_hda_codec(dev)); + > >>>>>> hda_codec_driver_remove > >>>>>> (dev_to_hda_codec(dev)); } > >>>>> Sorry, it's no-no.  The suspend can't be called unconditionally, and > >>>>> the driver unbind must not be called in the callback itself. > >>>>> > >>>>> Does the patch below work instead? > >>>> Sorry there was a typo.  A bit more revised patch is below. > >>>> > >>>> > >>>> Takashi > >>>> > >>>> --- a/sound/pci/hda/hda_intel.c > >>>> +++ b/sound/pci/hda/hda_intel.c > >>>> @@ -1383,14 +1383,17 @@ static void azx_free(struct azx *chip) > >>>>        hda->freed = 1; > >>>>    } > >>>>    -static int azx_dev_disconnect(struct snd_device *device) > >>>> +static void __azx_disconnect(struct azx *chip) > >>>>    { > >>>> -    struct azx *chip = device->device_data; > >>>>        struct hdac_bus *bus = azx_bus(chip); > >>>>          chip->bus.shutdown = 1; > >>>>        cancel_work_sync(&bus->unsol_work); > >>>> +} > >>>>    +static int azx_dev_disconnect(struct snd_device *device) > >>>> +{ > >>>> +    __azx_disconnect(device->device_data); > >>>>        return 0; > >>>>    } > >>>>    @@ -2356,8 +2359,10 @@ static void azx_shutdown(struct pci_dev > >>>> *pci) > >>>>        if (!card) > >>>>            return; > >>>>        chip = card->private_data; > >>>> -    if (chip && chip->running) > >>>> +    if (chip && chip->running) { > >>>> +        __azx_disconnect(chip); > >>>>            azx_shutdown_chip(chip); > >>>> +    } > >>>>    } > >>>>      /* PCI IDs */ > >>> Hi Takashi, > >>> > >>> Applied fix and tested on dolphin HW. Issue still there, here is > >>> captured screen on reboot from command line: > >>> > >>> reboot capture > >>> > >>> Reboot from UI works differently, no resume() call in this case. > >> Thanks for quick testing. > >> > >> After reconsideration, I believe we can even take a simpler path. > >> Use pm_runtime_force_suspend(), and keep suspended by > >> pm_runtime_disable() call afterwards. > >> > >> Below is another test patch.  Could you check whether this works > >> better? > >> > >> > >> Takashi > >> > >> --- a/sound/pci/hda/hda_codec.c > >> +++ b/sound/pci/hda/hda_codec.c > >> @@ -2986,13 +2986,11 @@ void snd_hda_codec_shutdown(struct > >> hda_codec *codec) > >>   { > >>       struct hda_pcm *cpcm; > >>   -    if (pm_runtime_suspended(hda_codec_dev(codec))) > >> -        return; > >> - > >>       list_for_each_entry(cpcm, &codec->pcm_list_head, list) > >>           snd_pcm_suspend_all(cpcm->pcm); > >>   -    pm_runtime_suspend(hda_codec_dev(codec)); > >> +    pm_runtime_force_suspend(hda_codec_dev(codec)); > >> +    pm_runtime_disable(hda_codec_dev(codec)); > >>   } > >>     /* > > > > Hi Takashi, > > > > Thank you very much! Yes, that works fine.  suspend() has been > > called on reboot and no pops. > > > > I still have previous patch in the code, so let me remove it and > > test it again with only snd_hda_codec_shutdown() changes. > > > > Thanks, > > > > Vitaly > > > > > Hi Takashi, > > I have finished regression tests on all our platforms and results are > positive, latest patch has fixed an issue with pops on reboot and no > > regression on other platforms as well. Great, thanks for testing again! I'll submit and merge the patch with your reported-and-tested-by tag. Takashi