Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 28 Aug 2018 19:05:51 +0200
Message-ID: <s5hsh2y5sls.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     "Anshuman Gupta" <anshuman.gupta@intel.com>
Cc:     <lgirdwood@gmail.com>, <puneethx.prabhu@intel.com>,
        <broonie@kernel.org>, <rafael@kernel.org>, <perex@perex.cz>,
        <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
        <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v2 1/1] hdac-codec runtime suspended at PM:Suspend.
In-Reply-To: <1535474055-11763-1-git-send-email-anshuman.gupta@intel.com>
References: <1534615923-8541-2-git-send-email-anshuman.gupta@intel.com>
        <1535474055-11763-1-git-send-email-anshuman.gupta@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/26
 (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

On Tue, 28 Aug 2018 18:34:15 +0200,
Anshuman Gupta wrote:
> 
> Is this patch not in consideration, there are no review comment for it. 
> this patch fixes an issue with hdac hdmi codec driver.
> 
> On one of our platform HD audio controller takes arounf 300ms.
> Below are the snippet of dmesg log.
> 
> [ 3778.461888] calling  0000:00:0e.0+ @ 3667, parent: pci0000:00, cb: pci_pm_resume
> [ 3778.775273] call 0000:00:0e.0+ returned 0 after 306016 usecs
> 
> When HD audio controller is in runtime suspend state, 
> with direct complete, we can improve overall system wide resume latency.

Actually, *this* should have been mentioned in the changelog, and the
subject would be better phrased to reflect it.

After all, you're trying to reduce / optimize the runtime PM latency.


thanks,

Takashi

> On Sat, Aug 18, 2018 at 11:42:03PM +0530, Anshuman Gupta wrote:
> > Keep hdac hdmi codec to be in runtime suspended while entering to
> > system wide suspend. Currently hdac hdmi codec driver using its
> > suspend and resume operation in prepare and complete PM callbacks,
> > and it resumes the hd audio controller (parent of self) from runtime
> > suspend and blocks the direct complete for hd audio controller.
> >
> > If hdac-codec is already in runtime suspend state skip its power down
> > sequence in prepare, power up sequence in complete phase. It enables
> > direct complete path for hdac-codec and hd audio controller
> > PCI device.
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > Changes in v2
> > - Changed the commit message.
> > - Using pm_runtime_suspended instead of pm_runtime_status_suspended
> >   in order to handle any race condition.
> >
> > sound/soc/codecs/hdac_hdmi.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> > index 84f7a7a..e965338 100644
> > --- a/sound/soc/codecs/hdac_hdmi.c
> > +++ b/sound/soc/codecs/hdac_hdmi.c
> > @@ -1859,6 +1859,9 @@ static int hdmi_codec_prepare(struct device *dev)
> >       struct hdac_ext_device *edev = to_hda_ext_device(dev);
> >       struct hdac_device *hdev = &edev->hdev;
> >
> > +     if (pm_runtime_suspended(dev))
> > +             return 1;
> > +
> >       pm_runtime_get_sync(&edev->hdev.dev);
> >
> >       /*
> > @@ -1880,6 +1883,9 @@ static void hdmi_codec_complete(struct device *dev)
> >       struct hdac_hdmi_priv *hdmi = hdev_to_hdmi_priv(&edev->hdev);
> >       struct hdac_device *hdev = &edev->hdev;
> >
> > +     if (pm_runtime_suspended(dev))
> > +             return;
> > +
> >       /* Power up afg */
> >       snd_hdac_codec_read(hdev, hdev->afg, 0, AC_VERB_SET_POWER_STATE,
> >                                                       AC_PWRST_D0);
> > --
> > 2.7.4
> >
> 
> --
> Thanks,
> Anshuman.
>  
>