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=-18.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 90B58C433ED for ; Wed, 7 Apr 2021 16:11:37 +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 D03EF611BE for ; Wed, 7 Apr 2021 16:11:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D03EF611BE 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 8D04415F2; Wed, 7 Apr 2021 18:10:43 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 8D04415F2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1617811893; bh=jAwd0t8ZPSlfjrmB1ZCS6Z2y+BNiqi64jFZ1YW2cIvo=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=MJhprdW7oFnjn5KA+A/wY5BNg/wBFhjBUCyM5jOYsw3kHqeFR6mXtEzLWPleythOI vSkfTY4K2+Y52nJ6gbbQXjsn3a+/pjhPsMe0Vu3bI8dgvIkBPlka8NLVzkVYFwhb7g svjKvZ0cirNhSzhUL0Dh/LQQgY+PL6R5eYWe1pas= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id EB757F800BD; Wed, 7 Apr 2021 18:10:42 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id B4744F80168; Wed, 7 Apr 2021 18:10:41 +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 53234F800BD for ; Wed, 7 Apr 2021 18:10:31 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 53234F800BD X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 97470B1AE; Wed, 7 Apr 2021 16:10:31 +0000 (UTC) Date: Wed, 07 Apr 2021 18:10:31 +0200 Message-ID: From: Takashi Iwai To: Kai Vehmanen Subject: Re: [RFC PATCH] ALSA: hda/hdmi: fix race in handling acomp ELD notification at resume In-Reply-To: <20210407154727.589017-1-kai.vehmanen@linux.intel.com> References: <20210407154727.589017-1-kai.vehmanen@linux.intel.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=US-ASCII Cc: alsa-devel@alsa-project.org, pierre-louis.bossart@linux.intel.com 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, 07 Apr 2021 17:47:27 +0200, Kai Vehmanen wrote: > > When snd-hda-codec-hdmi is used with ASoC HDA controller like SOF (acomp > used for ELD notifications), display connection change done during suspend, > can be lost due to following sequence of events: > > 1. system in S3 suspend > 2. DP/HDMI receiver connected > 3. system resumed > 4. HDA controller resumed, but card->deferred_resume_work not complete > 5. acomp eld_notify callback > 6. eld_notify ignored as power state is not CTL_POWER_D0 > 7. HDA resume deferred work completed, power state set to CTL_POWER_D0 > > This results in losing the notification, and the jack state reported to > user-space is not correct. Hrm, that's odd. The logic there is: there is a manual call of hdmi_present_sense() for each pin in the resume call back of HDMI codec driver, so at the point 7, update_eld() is invoked from hdmi_present_sense(), which notifies the state to user-space. So I don't see what's missing there. Could you check whether the scenario above is correct? The state is updated in snd_hdac_acomp_get_eld() call in sync_eld_via_acomp(). We can see what state is returned there at which timing. The only possible case I can think of now is that the graphics driver isn't ready for returning the right value at the HDMI codec resume. But this should have been covered by the device link... thanks, Takashi > The check on step 6 was added in commit 8ae743e82f0b ("ALSA: hda - Skip > ELD notification during system suspend"). It would seem with the deferred > resume logic in ASoC core, this check is not safe. > > Fix the issue by modifying the check to only skip ELD notification > processing if power state is D3 or deeper. This helps in the ASoC > controller case as card power state is set to D2 at start of > soc_resume_deferred(). > > BugLink: https://github.com/thesofproject/linux/issues/2825 > Signed-off-by: Kai Vehmanen > --- > sound/pci/hda/patch_hdmi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > NOTES: > - I wonder if there is a better way to check for system suspend > case than looking at snd_power_get_state() > - 'chip->pm_prepared' is one option, but this is not directly available > to codec drivers > - storing PM target is hda_codec_pm_prepare() is perhaps one option > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 5de3666a7101..a43df036db1d 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -2654,7 +2654,7 @@ static void generic_acomp_pin_eld_notify(void *audio_ptr, int port, int dev_id) > /* skip notification during system suspend (but not in runtime PM); > * the state will be updated at resume > */ > - if (snd_power_get_state(codec->card) != SNDRV_CTL_POWER_D0) > + if (snd_power_get_state(codec->card) >= SNDRV_CTL_POWER_D3) > return; > /* ditto during suspend/resume process itself */ > if (snd_hdac_is_in_pm(&codec->core)) > @@ -2840,7 +2840,7 @@ static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe) > /* skip notification during system suspend (but not in runtime PM); > * the state will be updated at resume > */ > - if (snd_power_get_state(codec->card) != SNDRV_CTL_POWER_D0) > + if (snd_power_get_state(codec->card) >= SNDRV_CTL_POWER_D3) > return; > /* ditto during suspend/resume process itself */ > if (snd_hdac_is_in_pm(&codec->core)) > > base-commit: 7dc53a38e4ac00d68943bab91deadc67f07d4a0b > -- > 2.31.0 >