Re: [PATCH v4 2/2] ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound to a driver

[Takashi Iwai]

On Mon, 07 Oct 2019 20:49:56 +0200,
Kai-Heng Feng wrote:
> 
> Hi Takashi,
> 
> > On Sep 25, 2019, at 19:32, Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote:
> > 
> > Nvidia proprietary driver doesn't support runtime power management, so
> > when a user only wants to use the integrated GPU, it's a common practice
> > to let dGPU not to bind any driver, and let its upstream port to be
> > runtime suspended. At the end of runtime suspension the port uses
> > platform power management to disable power through _OFF method of power
> > resource, which is listed by _PR3.
> > 
> > After commit b516ea586d71 ("PCI: Enable NVIDIA HDA controllers"), when
> > the dGPU comes with an HDA function, the HDA won't be suspended if the
> > dGPU is unbound, so the power resource can't be turned off by its
> > upstream port driver.
> > 
> > Commit 37a3a98ef601 ("ALSA: hda - Enable runtime PM only for
> > discrete GPU") only allows HDA to be runtime suspended once GPU is
> > bound, to keep APU's HDA working.
> > 
> > However, HDA on dGPU isn't that useful if dGPU is not bound to any
> > driver.  So let's relax the runtime suspend requirement for dGPU's HDA
> > function, to disable the power source to save lots of power.
> > 
> > BugLink: https://bugs.launchpad.net/bugs/1840835
> > Fixes: b516ea586d71 ("PCI: Enable NVIDIA HDA controllers")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> 
> Do you still have any concern on this patch?
> Please merge [v5 1/2] and this patch [v4 2/2] if you think it's good.

Sorry for the late reply, as I've been off for a few weeks.
Now I applied both patches for 5.4.


Thanks!

Takashi

> 
> Thanks!
> 
> Kai-Heng
> 
> > ---
> > v4:
> > - Find upstream port, it's callee's responsibility now.
> > v3:
> > - Make changelog more clear.
> > v2:
> > - Change wording.
> > - Rebase to Tiwai's branch.
> > sound/pci/hda/hda_intel.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 240f4ca76391..e63b871343e5 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -1280,11 +1280,17 @@ static void init_vga_switcheroo(struct azx *chip)
> > {
> > 	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
> > 	struct pci_dev *p = get_bound_vga(chip->pci);
> > +	struct pci_dev *parent;
> > 	if (p) {
> > 		dev_info(chip->card->dev,
> > 			 "Handle vga_switcheroo audio client\n");
> > 		hda->use_vga_switcheroo = 1;
> > -		chip->bus.keep_power = 1; /* cleared in either gpu_bound op or codec probe */
> > +
> > +		/* cleared in either gpu_bound op or codec probe, or when its
> > +		 * upstream port has _PR3 (i.e. dGPU).
> > +		 */
> > +		parent = pci_upstream_bridge(p);
> > +		chip->bus.keep_power = parent ? !pci_pr3_present(parent) : 1;
> > 		chip->driver_caps |= AZX_DCAPS_PM_RUNTIME;
> > 		pci_dev_put(p);
> > 	}
> > -- 
> > 2.17.1
> > 
> 
>