[SRU] [D/E/Unstable] [PATCH 0/3] Allow Nvidia HDA to be runtime suspended to save power

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [D/E/Unstable] [PATCH 0/3] Allow Nvidia HDA to be runtime suspended to save power

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1840835

[Impact]
Some Nvidia graphics card has audio function in addition to video
function. When the video function is not bound to any driver, the audio
function can't be runtime suspended, hence preventing the power
resources from being turned off.

[Fix]
Allow Nvidia audio function to be runtime suspended.

[Test]
The user reported the fix works.

[Regression Potential]
Low. It only allows the audio function of discrete Nvidia GPU to be
runtime suspended. Regular audio or audio function with a discrete GPU
already bound to a driver are unaffected.

Kai-Heng Feng (2):
  PCI: Add a helper to check Power Resource Requirements _PR3 existence
  ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound to
    a driver

Takashi Iwai (1):
  PCI: Fix missing inline for pci_pr3_present()

 drivers/pci/pci.c         | 18 ++++++++++++++++++
 include/linux/pci.h       |  2 ++
 sound/pci/hda/hda_intel.c |  8 +++++++-
 3 files changed, 27 insertions(+), 1 deletion(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[D/E/Unstable] [PATCH 1/3] PCI: Add a helper to check Power Resource Requirements _PR3 existence

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1840835

A driver may want to know the existence of _PR3, to choose different
runtime suspend behavior. A user will be add in next patch.

This is mostly the same as nouveau_pr3_present().

Signed-off-by: Kai-Heng Feng <[hidden email]>
Acked-by: Bjorn Helgaas <[hidden email]>
Link: https://lore.kernel.org/r/20191018073848.14590-1-kai.heng.feng@...
Signed-off-by: Takashi Iwai <[hidden email]>
(cherry picked from commit 52525b7a3cf82adec5c6cf0ecbd23ff228badc94 linux-next)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/pci/pci.c   | 18 ++++++++++++++++++
 include/linux/pci.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a97e2571a527..fcfaadc774ee 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5854,6 +5854,24 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
  return 0;
 }
 
+#ifdef CONFIG_ACPI
+bool pci_pr3_present(struct pci_dev *pdev)
+{
+ struct acpi_device *adev;
+
+ if (acpi_disabled)
+ return false;
+
+ adev = ACPI_COMPANION(&pdev->dev);
+ if (!adev)
+ return false;
+
+ return adev->power.flags.power_resources &&
+ acpi_has_method(adev->handle, "_PR3");
+}
+EXPORT_SYMBOL_GPL(pci_pr3_present);
+#endif
+
 /**
  * pci_add_dma_alias - Add a DMA devfn alias for a device
  * @dev: the PCI device for which alias is added
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f9088c89a534..1d15c5d49cdd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2310,9 +2310,11 @@ struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
 
 void
 pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
+bool pci_pr3_present(struct pci_dev *pdev);
 #else
 static inline struct irq_domain *
 pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
+static bool pci_pr3_present(struct pci_dev *pdev) { return false; }
 #endif
 
 #ifdef CONFIG_EEH
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1840835

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 <[hidden email]>
Link: https://lore.kernel.org/r/20191018073848.14590-2-kai.heng.feng@...
Signed-off-by: Takashi Iwai <[hidden email]>
(backported from commit bacd861452d2be86a4df341b12e32db7dac8021e linux-next)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 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 783f9a9c40ec..2f92709c4b26 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1281,11 +1281,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;
- hda->need_eld_notify_link = 1; /* cleared in gpu_bound op */
+
+ /* cleared in either gpu_bound op or codec probe, or when its
+ * upstream port has _PR3 (i.e. dGPU).
+ */
+ parent = pci_upstream_bridge(p);
+ hda->need_eld_notify_link = parent ? !pci_pr3_present(parent) : 1;
  chip->driver_caps |= AZX_DCAPS_PM_RUNTIME;
  pci_dev_put(p);
  }
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1840835

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 <[hidden email]>
Link: https://lore.kernel.org/r/20191018073848.14590-2-kai.heng.feng@...
Signed-off-by: Takashi Iwai <[hidden email]>
(cherry picked from commit bacd861452d2be86a4df341b12e32db7dac8021e linux-next)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[D/E/Unstable] [PATCH 3/3] PCI: Fix missing inline for pci_pr3_present()

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: Takashi Iwai <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1840835

The inline prefix was missing in the dummy function pci_pr3_present()
definition.  Fix it.

Reported-by: kbuild test robot <[hidden email]>
Fixes: 52525b7a3cf8 ("PCI: Add a helper to check Power Resource Requirements _PR3 existence")
Link: <a href="https://lore.kernel.org/r/201910212111.qHm6OcWx%lkp@intel.com">https://lore.kernel.org/r/201910212111.qHm6OcWx%lkp@...
Signed-off-by: Takashi Iwai <[hidden email]>
(cherry picked from commit 46b4bff6572b0552b1ee062043621e4b252638d8 linux-next)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 include/linux/pci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1d15c5d49cdd..be529d311122 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2314,7 +2314,7 @@ bool pci_pr3_present(struct pci_dev *pdev);
 #else
 static inline struct irq_domain *
 pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
-static bool pci_pr3_present(struct pci_dev *pdev) { return false; }
+static inline bool pci_pr3_present(struct pci_dev *pdev) { return false; }
 #endif
 
 #ifdef CONFIG_EEH
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK / APPLIED[Unstable]: [SRU] [D/E/Unstable] [PATCH 0/3] Allow Nvidia HDA to be runtime suspended to save power

Seth Forshee
In reply to this post by Kai-Heng Feng
On Wed, Oct 23, 2019 at 10:25:24PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1840835
>
> [Impact]
> Some Nvidia graphics card has audio function in addition to video
> function. When the video function is not bound to any driver, the audio
> function can't be runtime suspended, hence preventing the power
> resources from being turned off.
>
> [Fix]
> Allow Nvidia audio function to be runtime suspended.
>
> [Test]
> The user reported the fix works.
>
> [Regression Potential]
> Low. It only allows the audio function of discrete Nvidia GPU to be
> runtime suspended. Regular audio or audio function with a discrete GPU
> already bound to a driver are unaffected.

Acked-by: Seth Forshee <[hidden email]>

Applied to unstable/master, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [D/E] [PATCH 0/3] Allow Nvidia HDA to be runtime suspended to save power

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 23.10.19 16:25, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1840835
>
> [Impact]
> Some Nvidia graphics card has audio function in addition to video
> function. When the video function is not bound to any driver, the audio
> function can't be runtime suspended, hence preventing the power
> resources from being turned off.
>
> [Fix]
> Allow Nvidia audio function to be runtime suspended.
>
> [Test]
> The user reported the fix works.
>
> [Regression Potential]
> Low. It only allows the audio function of discrete Nvidia GPU to be
> runtime suspended. Regular audio or audio function with a discrete GPU
> already bound to a driver are unaffected.
>
> Kai-Heng Feng (2):
>   PCI: Add a helper to check Power Resource Requirements _PR3 existence
>   ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound to
>     a driver
>
> Takashi Iwai (1):
>   PCI: Fix missing inline for pci_pr3_present()
>
>  drivers/pci/pci.c         | 18 ++++++++++++++++++
>  include/linux/pci.h       |  2 ++
>  sound/pci/hda/hda_intel.c |  8 +++++++-
>  3 files changed, 27 insertions(+), 1 deletion(-)
>
Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED(D,E): [SRU] [D/E/Unstable] [PATCH 0/3] Allow Nvidia HDA to be runtime suspended to save power

Khaled Elmously
In reply to this post by Kai-Heng Feng
On 2019-10-23 22:25:24 , Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1840835
>
> [Impact]
> Some Nvidia graphics card has audio function in addition to video
> function. When the video function is not bound to any driver, the audio
> function can't be runtime suspended, hence preventing the power
> resources from being turned off.
>
> [Fix]
> Allow Nvidia audio function to be runtime suspended.
>
> [Test]
> The user reported the fix works.
>
> [Regression Potential]
> Low. It only allows the audio function of discrete Nvidia GPU to be
> runtime suspended. Regular audio or audio function with a discrete GPU
> already bound to a driver are unaffected.
>
> Kai-Heng Feng (2):
>   PCI: Add a helper to check Power Resource Requirements _PR3 existence
>   ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound to
>     a driver
>
> Takashi Iwai (1):
>   PCI: Fix missing inline for pci_pr3_present()
>
>  drivers/pci/pci.c         | 18 ++++++++++++++++++
>  include/linux/pci.h       |  2 ++
>  sound/pci/hda/hda_intel.c |  8 +++++++-
>  3 files changed, 27 insertions(+), 1 deletion(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team