From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by alsa0.perex.cz (Postfix) with ESMTP id BA5D12670F8 for ; Wed, 14 Mar 2018 11:39:38 +0100 (CET) Date: Wed, 14 Mar 2018 16:08:16 +0530 From: Anshuman Gupta Message-ID: <20180314103816.GA7012@anshuman.gupta@intel.com> References: <1520853467-31653-1-git-send-email-anshuman.gupta@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Subject: Re: [alsa-devel] [PATCH] [sound] hdac-codec runtime suspended at PM:Suspend. List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: "Rafael J. Wysocki" Cc: "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , Liam Girdwood , Linux PM , Linux Kernel Mailing List , Takashi Iwai , Mark Brown List-ID: On Mon, Mar 12, 2018 at 12:26:53PM +0100, Rafael J. Wysocki wrote: > On Mon, Mar 12, 2018 at 12:17 PM, Anshuman Gupta > wrote: > > Keep hdac-codec to be in runtime suspended while entering to suspend. > > If hdac-codec is already in runtime suspend state skip its power down > > sequence in prepare, power up sequence in complete phase. > > > > Avoid resuming hdac controller PCI device 00:1f.3 from runtime suspend > > state in case hdac-codec already in runtime-suspend state, this is > > unnecessary and block the direct complete even for hdac controller > > PCI device 00:1f.3. > > > > This enabled direct complete path for hdac-codec and PCI device 00:1f.3. > > > > Signed-off-by: Anshuman Gupta > > --- > > sound/soc/codecs/hdac_hdmi.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c > > index f3b4f4d..810a8a6 100644 > > --- a/sound/soc/codecs/hdac_hdmi.c > > +++ b/sound/soc/codecs/hdac_hdmi.c > > @@ -1852,6 +1852,8 @@ static int hdmi_codec_prepare(struct device *dev) > > struct hdac_ext_device *edev = to_hda_ext_device(dev); > > struct hdac_device *hdac = &edev->hdac; > > > > + if (pm_runtime_status_suspended(dev)) > > + return 1; > > This is racy in principle, because the runtime PM status can still > change after you've checked here. Will using pm_runtime_disable/pm_runtime_enable for safe check of runtime status avoids this race? > > But even if it isn't racy in practice, the following > pm_runtime_get_sync() becomes redundant after it, doesn't it? I have no idea but if there can be a case that PCI 00:1f.3 (hdac controller) is runtime suspended and hdac hdmi codec is active, in that case it may be required to use pm_runtime_get_sync() for hdac controller as it is parent of hdac hdmi codec. > > > pm_runtime_get_sync(&edev->hdac.dev); > > > > /* > > @@ -1873,6 +1875,8 @@ static void hdmi_codec_complete(struct device *dev) > > struct hdac_hdmi_priv *hdmi = edev->private_data; > > struct hdac_device *hdac = &edev->hdac; > > > > + if (pm_runtime_status_suspended(dev)) > > + return; > > That, again, is somewhat fragile from the concurrency perspective. > > > /* Power up afg */ > > snd_hdac_codec_read(hdac, hdac->afg, 0, AC_VERB_SET_POWER_STATE, > > AC_PWRST_D0); > > -- > > 2.7.4 > > -- Thanks, Anshuman _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel