Nouveau - Jul 2016 - [PATCH v3 1/4] drm/nouveau/acpi: ensure matching ACPI handle and supported functions

Hi,

Here are two patches to fix an issue reported on kernel bugzilla (infinite loop
due to unchecked function) and a more important fix to fix hanging Optimus
machines when runtime PM is enabled (with pm/pci patches).

These are the final patches targeting v4.8. Changes compared to v2[1]:
collected R-b from Hans and Mika and fixed a minor comment style issue.

I recommend it to be merged before the pci/pm patches[2], otherwise there is a
window where newer Nvidia Optimus laptops might fail to runtime resume and/or
lock up.  Once the pci/pm branch is merged I will propose another patch to
improve reliability[3].

Known issue with patch 4: when a Nvidia HDMI audio function is present, the
bridge will not suspend and hence the Nvidia card will still be powered. Fixing
this properly will require more work[4], until then you can kill the audio
device and make runtime PM work properly:

    echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove

Kind regards,
Peter

 [1]: https://lists.freedesktop.org/archives/nouveau/2016-July/025519.html
 [2]: https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/?h=pci/pm
 [3]: http://www.spinics.net/lists/linux-pci/msg52601.html
 [4]: https://lists.freedesktop.org/archives/dri-devel/2016-July/112759.html

Peter Wu (4):
  drm/nouveau/acpi: ensure matching ACPI handle and supported functions
  drm/nouveau/acpi: return supported DSM functions
  drm/nouveau/acpi: check for function 0x1B before using it
  drm/nouveau/acpi: fix lockup with PCIe runtime PM

 drivers/gpu/drm/nouveau/nouveau_acpi.c | 105 +++++++++++++++++++++------------
 1 file changed, 68 insertions(+), 37 deletions(-)

-- 
2.9.0

Ensure that the returned set of supported DSM functions (MUX, Optimus)
match the ACPI handle that is set in nouveau_dsm_pci_probe.

As there are no machines with a MUX function on just one PCI device and
an Optimus on another, there should not be a functional impact. This
change however makes this implicit assumption more obvious.

Convert int to bool and rename has_dsm to has_mux while at it. Let the
caller set nouveau_dsm_priv.dhandle as needed.

 v2: pass dhandle to the caller.

Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 58 +++++++++++++++-------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index db76b94..886a67c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -57,9 +57,6 @@ bool nouveau_is_v1_dsm(void) {
 	return nouveau_dsm_priv.dsm_detected;
 }
 
-#define NOUVEAU_DSM_HAS_MUX 0x1
-#define NOUVEAU_DSM_HAS_OPT 0x2
-
 #ifdef CONFIG_VGA_SWITCHEROO
 static const char nouveau_dsm_muid[] = {
 	0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
@@ -212,26 +209,33 @@ static const struct vga_switcheroo_handler
nouveau_dsm_handler = {
 	.get_client_id = nouveau_dsm_get_client_id,
 };
 
-static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
+static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle
*dhandle_out,
+				  bool *has_mux, bool *has_opt)
 {
 	acpi_handle dhandle;
-	int retval = 0;
+	bool supports_mux;
+	bool supports_opt;
 
 	dhandle = ACPI_HANDLE(&pdev->dev);
 	if (!dhandle)
-		return false;
+		return;
 
 	if (!acpi_has_method(dhandle, "_DSM"))
-		return false;
+		return;
 
-	if (acpi_check_dsm(dhandle, nouveau_dsm_muid, 0x00000102,
-			   1 << NOUVEAU_DSM_POWER))
-		retval |= NOUVEAU_DSM_HAS_MUX;
+	supports_mux = acpi_check_dsm(dhandle, nouveau_dsm_muid, 0x00000102,
+				      1 << NOUVEAU_DSM_POWER);
+	supports_opt = nouveau_check_optimus_dsm(dhandle);
 
-	if (nouveau_check_optimus_dsm(dhandle))
-		retval |= NOUVEAU_DSM_HAS_OPT;
+	/* Does not look like a Nvidia device. */
+	if (!supports_mux && !supports_opt)
+		return;
 
-	if (retval & NOUVEAU_DSM_HAS_OPT) {
+	*dhandle_out = dhandle;
+	*has_mux = supports_mux;
+	*has_opt = supports_opt;
+
+	if (supports_opt) {
 		uint32_t result;
 		nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
 				    &result);
@@ -240,10 +244,6 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
 			 (result & OPTIMUS_DYNAMIC_PWR_CAP) ? "dynamic power, " :
"",
 			 (result & OPTIMUS_HDA_CODEC_MASK) ? "hda bios codec
supported" : "");
 	}
-	if (retval)
-		nouveau_dsm_priv.dhandle = dhandle;
-
-	return retval;
 }
 
 static bool nouveau_dsm_detect(void)
@@ -251,11 +251,11 @@ static bool nouveau_dsm_detect(void)
 	char acpi_method_name[255] = { 0 };
 	struct acpi_buffer buffer = {sizeof(acpi_method_name), acpi_method_name};
 	struct pci_dev *pdev = NULL;
-	int has_dsm = 0;
-	int has_optimus = 0;
+	acpi_handle dhandle = NULL;
+	bool has_mux = false;
+	bool has_optimus = false;
 	int vga_count = 0;
 	bool guid_valid;
-	int retval;
 	bool ret = false;
 
 	/* lookup the MXM GUID */
@@ -268,32 +268,26 @@ static bool nouveau_dsm_detect(void)
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		retval = nouveau_dsm_pci_probe(pdev);
-		if (retval & NOUVEAU_DSM_HAS_MUX)
-			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
+		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus);
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		retval = nouveau_dsm_pci_probe(pdev);
-		if (retval & NOUVEAU_DSM_HAS_MUX)
-			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
+		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus);
 	}
 
 	/* find the optimus DSM or the old v1 DSM */
-	if (has_optimus == 1) {
+	if (has_optimus) {
+		nouveau_dsm_priv.dhandle = dhandle;
 		acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
 			&buffer);
 		printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method %s
handle\n",
 			acpi_method_name);
 		nouveau_dsm_priv.optimus_detected = true;
 		ret = true;
-	} else if (vga_count == 2 && has_dsm && guid_valid) {
+	} else if (vga_count == 2 && has_mux && guid_valid) {
+		nouveau_dsm_priv.dhandle = dhandle;
 		acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
 			&buffer);
 		printk(KERN_INFO "VGA switcheroo: detected DSM switching method %s
handle\n",
-- 
2.9.0

Return the set of supported functions to the caller. No functional
changes.

Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index 886a67c..572ac30 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -107,7 +107,7 @@ static int nouveau_optimus_dsm(acpi_handle handle, int func,
int arg, uint32_t *
  * requirements on the fourth parameter, so a private implementation
  * instead of using acpi_check_dsm().
  */
-static int nouveau_check_optimus_dsm(acpi_handle handle)
+static int nouveau_dsm_get_optimus_functions(acpi_handle handle)
 {
 	int result;
 
@@ -122,7 +122,9 @@ static int nouveau_check_optimus_dsm(acpi_handle handle)
 	 * ACPI Spec v4 9.14.1: if bit 0 is zero, no function is supported.
 	 * If the n-th bit is enabled, function n is supported
 	 */
-	return result & 1 && result & (1 <<
NOUVEAU_DSM_OPTIMUS_CAPS);
+	if (result & 1 && result & (1 <<
NOUVEAU_DSM_OPTIMUS_CAPS))
+		return result;
+	return 0;
 }
 
 static int nouveau_dsm(acpi_handle handle, int func, int arg)
@@ -214,7 +216,7 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev,
acpi_handle *dhandle_out
 {
 	acpi_handle dhandle;
 	bool supports_mux;
-	bool supports_opt;
+	int optimus_funcs;
 
 	dhandle = ACPI_HANDLE(&pdev->dev);
 	if (!dhandle)
@@ -225,17 +227,17 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev,
acpi_handle *dhandle_out
 
 	supports_mux = acpi_check_dsm(dhandle, nouveau_dsm_muid, 0x00000102,
 				      1 << NOUVEAU_DSM_POWER);
-	supports_opt = nouveau_check_optimus_dsm(dhandle);
+	optimus_funcs = nouveau_dsm_get_optimus_functions(dhandle);
 
 	/* Does not look like a Nvidia device. */
-	if (!supports_mux && !supports_opt)
+	if (!supports_mux && !optimus_funcs)
 		return;
 
 	*dhandle_out = dhandle;
 	*has_mux = supports_mux;
-	*has_opt = supports_opt;
+	*has_opt = !!optimus_funcs;
 
-	if (supports_opt) {
+	if (optimus_funcs) {
 		uint32_t result;
 		nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
 				    &result);
-- 
2.9.0

Do not unconditionally invoke function 0x1B without checking for its
availability, it leads to an infinite loop on some firmware.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=104791
Fixes: 5addcf0a5f0fad ("nouveau: add runtime PM support (v0.9)")
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index 572ac30..ad273ad 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -45,6 +45,7 @@
 static struct nouveau_dsm_priv {
 	bool dsm_detected;
 	bool optimus_detected;
+	bool optimus_flags_detected;
 	acpi_handle dhandle;
 	acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -212,7 +213,8 @@ static const struct vga_switcheroo_handler
nouveau_dsm_handler = {
 };
 
 static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle
*dhandle_out,
-				  bool *has_mux, bool *has_opt)
+				  bool *has_mux, bool *has_opt,
+				  bool *has_opt_flags)
 {
 	acpi_handle dhandle;
 	bool supports_mux;
@@ -236,6 +238,7 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev,
acpi_handle *dhandle_out
 	*dhandle_out = dhandle;
 	*has_mux = supports_mux;
 	*has_opt = !!optimus_funcs;
+	*has_opt_flags = optimus_funcs & (1 << NOUVEAU_DSM_OPTIMUS_FLAGS);
 
 	if (optimus_funcs) {
 		uint32_t result;
@@ -256,6 +259,7 @@ static bool nouveau_dsm_detect(void)
 	acpi_handle dhandle = NULL;
 	bool has_mux = false;
 	bool has_optimus = false;
+	bool has_optimus_flags = false;
 	int vga_count = 0;
 	bool guid_valid;
 	bool ret = false;
@@ -270,13 +274,15 @@ static bool nouveau_dsm_detect(void)
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus);
+		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus,
+				      &has_optimus_flags);
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus);
+		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus,
+				      &has_optimus_flags);
 	}
 
 	/* find the optimus DSM or the old v1 DSM */
@@ -287,6 +293,7 @@ static bool nouveau_dsm_detect(void)
 		printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method %s
handle\n",
 			acpi_method_name);
 		nouveau_dsm_priv.optimus_detected = true;
+		nouveau_dsm_priv.optimus_flags_detected = has_optimus_flags;
 		ret = true;
 	} else if (vga_count == 2 && has_mux && guid_valid) {
 		nouveau_dsm_priv.dhandle = dhandle;
@@ -320,8 +327,9 @@ void nouveau_switcheroo_optimus_dsm(void)
 	if (!nouveau_dsm_priv.optimus_detected)
 		return;
 
-	nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_FLAGS,
-			    0x3, &result);
+	if (nouveau_dsm_priv.optimus_flags_detected)
+		nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_FLAGS,
+				    0x3, &result);
 
 	nouveau_optimus_dsm(nouveau_dsm_priv.dhandle, NOUVEAU_DSM_OPTIMUS_CAPS,
 		NOUVEAU_DSM_OPTIMUS_SET_POWERDOWN, &result);
-- 
2.9.0

Since "PCI: Add runtime PM support for PCIe ports", the parent PCIe
port
can be runtime-suspended which disables power resources via ACPI. This
is incompatible with DSM, resulting in a GPU device which is still in D3
and locks up the kernel on resume (on a Clevo P651RA, GTX965M).

Mirror the behavior of Windows 8 and newer[1] (as observed via an AMLi
debugger trace) and stop using the DSM functions for D3cold when power
resources are available on the parent PCIe port.

pci_d3cold_disable() is not used because on some machines, the old DSM
method is broken. On a Lenovo T440p (GT 730M) memory and disk corruption
would occur, but that is fixed with this patch[2].

 [1]:
https://msdn.microsoft.com/windows/hardware/drivers/bringup/firmware-requirements-for-d3cold
 [2]:
https://github.com/Bumblebee-Project/bbswitch/issues/78#issuecomment-223549072

 v2: simply check directly for _PR3. Added affected machines.
 v3: fixed block comment coding style.

Reviewed-by: Mika Westerberg <mika.westerberg at linux.intel.com>
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 35 ++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index ad273ad..f2ad17a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -46,6 +46,7 @@ static struct nouveau_dsm_priv {
 	bool dsm_detected;
 	bool optimus_detected;
 	bool optimus_flags_detected;
+	bool optimus_skip_dsm;
 	acpi_handle dhandle;
 	acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -212,9 +213,28 @@ static const struct vga_switcheroo_handler
nouveau_dsm_handler = {
 	.get_client_id = nouveau_dsm_get_client_id,
 };
 
+/*
+ * Firmware supporting Windows 8 or later do not use _DSM to put the device
into
+ * D3cold, they instead rely on disabling power resources on the parent.
+ */
+static bool nouveau_pr3_present(struct pci_dev *pdev)
+{
+	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
+	struct acpi_device *parent_adev;
+
+	if (!parent_pdev)
+		return false;
+
+	parent_adev = ACPI_COMPANION(&parent_pdev->dev);
+	if (!parent_adev)
+		return false;
+
+	return acpi_has_method(parent_adev->handle, "_PR3");
+}
+
 static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle
*dhandle_out,
 				  bool *has_mux, bool *has_opt,
-				  bool *has_opt_flags)
+				  bool *has_opt_flags, bool *has_pr3)
 {
 	acpi_handle dhandle;
 	bool supports_mux;
@@ -239,6 +259,7 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev,
acpi_handle *dhandle_out
 	*has_mux = supports_mux;
 	*has_opt = !!optimus_funcs;
 	*has_opt_flags = optimus_funcs & (1 << NOUVEAU_DSM_OPTIMUS_FLAGS);
+	*has_pr3 = false;
 
 	if (optimus_funcs) {
 		uint32_t result;
@@ -248,6 +269,8 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev,
acpi_handle *dhandle_out
 			 (result & OPTIMUS_ENABLED) ? "enabled" :
"disabled",
 			 (result & OPTIMUS_DYNAMIC_PWR_CAP) ? "dynamic power, " :
"",
 			 (result & OPTIMUS_HDA_CODEC_MASK) ? "hda bios codec
supported" : "");
+
+		*has_pr3 = nouveau_pr3_present(pdev);
 	}
 }
 
@@ -260,6 +283,7 @@ static bool nouveau_dsm_detect(void)
 	bool has_mux = false;
 	bool has_optimus = false;
 	bool has_optimus_flags = false;
+	bool has_power_resources = false;
 	int vga_count = 0;
 	bool guid_valid;
 	bool ret = false;
@@ -275,14 +299,14 @@ static bool nouveau_dsm_detect(void)
 		vga_count++;
 
 		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus,
-				      &has_optimus_flags);
+				      &has_optimus_flags, &has_power_resources);
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL)
{
 		vga_count++;
 
 		nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus,
-				      &has_optimus_flags);
+				      &has_optimus_flags, &has_power_resources);
 	}
 
 	/* find the optimus DSM or the old v1 DSM */
@@ -292,8 +316,11 @@ static bool nouveau_dsm_detect(void)
 			&buffer);
 		printk(KERN_INFO "VGA switcheroo: detected Optimus DSM method %s
handle\n",
 			acpi_method_name);
+		if (has_power_resources)
+			pr_info("nouveau: detected PR support, will not use DSM\n");
 		nouveau_dsm_priv.optimus_detected = true;
 		nouveau_dsm_priv.optimus_flags_detected = has_optimus_flags;
+		nouveau_dsm_priv.optimus_skip_dsm = has_power_resources;
 		ret = true;
 	} else if (vga_count == 2 && has_mux && guid_valid) {
 		nouveau_dsm_priv.dhandle = dhandle;
@@ -324,7 +351,7 @@ void nouveau_register_dsm_handler(void)
 void nouveau_switcheroo_optimus_dsm(void)
 {
 	u32 result = 0;
-	if (!nouveau_dsm_priv.optimus_detected)
+	if (!nouveau_dsm_priv.optimus_detected || nouveau_dsm_priv.optimus_skip_dsm)
 		return;
 
 	if (nouveau_dsm_priv.optimus_flags_detected)
-- 
2.9.0

On Fri, Jul 15, 2016 at 9:12 AM, Peter Wu <peter at lekensteyn.nl>
wrote:
> Hi,
>
> Here are two patches to fix an issue reported on kernel bugzilla (infinite
loop
> due to unchecked function) and a more important fix to fix hanging Optimus
> machines when runtime PM is enabled (with pm/pci patches).
>
> These are the final patches targeting v4.8. Changes compared to v2[1]:
> collected R-b from Hans and Mika and fixed a minor comment style issue.
>
> I recommend it to be merged before the pci/pm patches[2], otherwise there
is a
> window where newer Nvidia Optimus laptops might fail to runtime resume
and/or
> lock up.  Once the pci/pm branch is merged I will propose another patch to
> improve reliability[3].
>
> Known issue with patch 4: when a Nvidia HDMI audio function is present, the
> bridge will not suspend and hence the Nvidia card will still be powered.
Fixing
That's basically all optimus gpu's, right? Anything GT21x+ has a HDMI
audio subfunction, and prior to that, the nvidia gpu tended to be the
only gpu, or hard-muxed.

If that's the case, that's pretty much a non-starter, IMO.

  -ilia

On Fri, Jul 15, 2016 at 12:10 PM, Ilia Mirkin <imirkin at alum.mit.edu>
wrote:
> On Fri, Jul 15, 2016 at 9:12 AM, Peter Wu <peter at lekensteyn.nl>
wrote:
>> Hi,
>>
>> Here are two patches to fix an issue reported on kernel bugzilla
(infinite loop
>> due to unchecked function) and a more important fix to fix hanging
Optimus
>> machines when runtime PM is enabled (with pm/pci patches).
>>
>> These are the final patches targeting v4.8. Changes compared to v2[1]:
>> collected R-b from Hans and Mika and fixed a minor comment style issue.
>>
>> I recommend it to be merged before the pci/pm patches[2], otherwise
there is a
>> window where newer Nvidia Optimus laptops might fail to runtime resume
and/or
>> lock up.  Once the pci/pm branch is merged I will propose another patch
to
>> improve reliability[3].
>>
>> Known issue with patch 4: when a Nvidia HDMI audio function is present,
the
>> bridge will not suspend and hence the Nvidia card will still be
powered. Fixing
>
> That's basically all optimus gpu's, right? Anything GT21x+ has a
HDMI
> audio subfunction, and prior to that, the nvidia gpu tended to be the
> only gpu, or hard-muxed.
I'd imagine there is a strap on the asic so the audio function is only
enabled if there is a connector actually wired to the dGPU.  It
doesn't make much sense to expose audio if there is no way to use it.

Alex

On Fri, Jul 15, 2016 at 12:10:23PM -0400, Ilia Mirkin
wrote:
> On Fri, Jul 15, 2016 at 9:12 AM, Peter Wu <peter at lekensteyn.nl>
wrote:
> > Hi,
> >
> > Here are two patches to fix an issue reported on kernel bugzilla
(infinite loop
> > due to unchecked function) and a more important fix to fix hanging
Optimus
> > machines when runtime PM is enabled (with pm/pci patches).
> >
> > These are the final patches targeting v4.8. Changes compared to v2[1]:
> > collected R-b from Hans and Mika and fixed a minor comment style
issue.
> >
> > I recommend it to be merged before the pci/pm patches[2], otherwise
there is a
> > window where newer Nvidia Optimus laptops might fail to runtime resume
and/or
> > lock up.  Once the pci/pm branch is merged I will propose another
patch to
> > improve reliability[3].
> >
> > Known issue with patch 4: when a Nvidia HDMI audio function is
present, the
> > bridge will not suspend and hence the Nvidia card will still be
powered. Fixing
> 
> That's basically all optimus gpu's, right? Anything GT21x+ has a
HDMI
> audio subfunction, and prior to that, the nvidia gpu tended to be the
> only gpu, or hard-muxed.
> 
> If that's the case, that's pretty much a non-starter, IMO.
For some reason the audio function tends to disappear/hide, so maybe it
is not as problematic as it appears (see
https://bugs.freedesktop.org/show_bug.cgi?id=75985). For my laptop I
also had to runtime suspend/resume before lspci -H1 shows the device,
loading with runpm=0 didn't return my HDMI audio device.

The powered on issue will also only appear on devices produced in 2013
and newer that happen to have this ACPI _PR3 ACPI method (which is quite
common for new machines supporting Windows 8 though).

For these newer laptops, after the pci/pm merge and after a patch like
http://www.spinics.net/lists/linux-pci/msg52601.html, the user can
revert to the old DSM method by booting with pcie_port_pm=off which will
retain the current behavior.

The advantage of this patch is that it fixes memory corruption on some
devices. The risk is that the card stays on because the audio subsystem
needs some more work.  FWIW, I was working on some patches that properly
suspended in presence of the HDA controller, but somehow the audio
device was not properly resumed resulting in "no AFG or MFG node
found"
and "snd_hda_intel 0000:01:00.1: no codecs initialized".
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl

Ping, would it be possible to get some acks and merge it for 4.8?
Current -next is broken (on modern laptops as expected) and these series
fix the issues according to an IRC report.

The audio issue mentioned below should not give issues, modern laptops
do not seem to expose the audio device by default
(https://bugs.freedesktop.org/show_bug.cgi?id=75985). On Windows the
audio device only appears after inserting the HDMI/miniDP cable (my
laptop has no other connectors), enabling the card for rendering
purposes has no effect on the availability of the audio device.

Kind regards,
Peter

On Fri, Jul 15, 2016 at 03:12:14PM +0200, Peter Wu
wrote:
> Hi,
> 
> Here are two patches to fix an issue reported on kernel bugzilla (infinite
loop
> due to unchecked function) and a more important fix to fix hanging Optimus
> machines when runtime PM is enabled (with pm/pci patches).
> 
> These are the final patches targeting v4.8. Changes compared to v2[1]:
> collected R-b from Hans and Mika and fixed a minor comment style issue.
> 
> I recommend it to be merged before the pci/pm patches[2], otherwise there
is a
> window where newer Nvidia Optimus laptops might fail to runtime resume
and/or
> lock up.  Once the pci/pm branch is merged I will propose another patch to
> improve reliability[3].
> 
> Known issue with patch 4: when a Nvidia HDMI audio function is present, the
> bridge will not suspend and hence the Nvidia card will still be powered.
Fixing
> this properly will require more work[4], until then you can kill the audio
> device and make runtime PM work properly:
> 
>     echo 1 > /sys/bus/pci/devices/0000:01:00.1/remove
> 
> Kind regards,
> Peter
> 
>  [1]: https://lists.freedesktop.org/archives/nouveau/2016-July/025519.html
>  [2]:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/?h=pci/pm
>  [3]: http://www.spinics.net/lists/linux-pci/msg52601.html
>  [4]:
https://lists.freedesktop.org/archives/dri-devel/2016-July/112759.html
> 
> Peter Wu (4):
>   drm/nouveau/acpi: ensure matching ACPI handle and supported functions
>   drm/nouveau/acpi: return supported DSM functions
>   drm/nouveau/acpi: check for function 0x1B before using it
>   drm/nouveau/acpi: fix lockup with PCIe runtime PM
> 
>  drivers/gpu/drm/nouveau/nouveau_acpi.c | 105
+++++++++++++++++++++------------
>  1 file changed, 68 insertions(+), 37 deletions(-)
> 
> -- 
> 2.9.0