Nouveau - May 2016 - [PATCH] drm/nouveau: check function before using it

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)")
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
Hi,

I am not sure what exactly 0x1B is used for, it seems related to HDMI (Audio?).
Bit0 of Arg3 is "OPFL", bit 1 is "OPVL" according to some
firmware.

On some laptops (P651RA) it sets "SGFL" (Switchable Graphics FLags?),
on others
it has no side-effects but returns a value based on Arg3 (Clevo P155M).
A Medion P7624 stores values to "OPTF" (OPTimus Flags?) and if this
flag is
true, \_SB.PCI0.PEG0.PEGP._ON will write zero to "NHDM".

While NOUVEAU_DSM_HAS_OPT is removed from this patch, I kept the flags return
value which can be useful for other probe outcomes (such as whether _PR3 is
found on the parent device).

Kinds regards,
Peter
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 41 +++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index cdf5227..002371a 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 flags_func_detected;
 	acpi_handle dhandle;
 	acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -58,7 +59,6 @@ bool nouveau_is_v1_dsm(void) {
 }
 
 #define NOUVEAU_DSM_HAS_MUX 0x1
-#define NOUVEAU_DSM_HAS_OPT 0x2
 
 #ifdef CONFIG_VGA_SWITCHEROO
 static const char nouveau_dsm_muid[] = {
@@ -110,9 +110,9 @@ 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 uint32_t nouveau_check_optimus_dsm(acpi_handle handle)
 {
-	int result;
+	uint32_t result;
 
 	/*
 	 * Function 0 returns a Buffer containing available functions.
@@ -123,9 +123,13 @@ 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
+	 * If the n-th bit is enabled, function n is supported.
+	 * Check for both bit zero and the NOUVEAU_DSM_OPTIMUS_CAPS since
+	 * some implementations return 0x80000001 on invalid parameters.
 	 */
-	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)
@@ -212,7 +216,7 @@ 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 int nouveau_dsm_pci_probe(struct pci_dev *pdev, uint32_t *optimus_funcs)
 {
 	acpi_handle dhandle;
 	int retval = 0;
@@ -228,10 +232,8 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
 			   1 << NOUVEAU_DSM_POWER))
 		retval |= NOUVEAU_DSM_HAS_MUX;
 
-	if (nouveau_check_optimus_dsm(dhandle))
-		retval |= NOUVEAU_DSM_HAS_OPT;
-
-	if (retval & NOUVEAU_DSM_HAS_OPT) {
+	*optimus_funcs = nouveau_check_optimus_dsm(dhandle);
+	if (*optimus_funcs) {
 		uint32_t result;
 		nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
 				    &result);
@@ -252,7 +254,7 @@ static bool nouveau_dsm_detect(void)
 	struct acpi_buffer buffer = {sizeof(acpi_method_name), acpi_method_name};
 	struct pci_dev *pdev = NULL;
 	int has_dsm = 0;
-	int has_optimus = 0;
+	uint32_t optimus_funcs = 0;
 	int vga_count = 0;
 	bool guid_valid;
 	int retval;
@@ -268,30 +270,28 @@ 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);
+		retval = nouveau_dsm_pci_probe(pdev, &optimus_funcs);
 		if (retval & NOUVEAU_DSM_HAS_MUX)
 			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		retval = nouveau_dsm_pci_probe(pdev);
+		retval = nouveau_dsm_pci_probe(pdev, &optimus_funcs);
 		if (retval & NOUVEAU_DSM_HAS_MUX)
 			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
 	}
 
 	/* find the optimus DSM or the old v1 DSM */
-	if (has_optimus == 1) {
+	if (optimus_funcs) {
 		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;
+		nouveau_dsm_priv.flags_func_detected +			optimus_funcs & (1 <<
NOUVEAU_DSM_OPTIMUS_FLAGS);
 		ret = true;
 	} else if (vga_count == 2 && has_dsm && guid_valid) {
 		acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
@@ -324,8 +324,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.flags_func_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.8.2

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)")
Signed-off-by: Peter Wu <peter at lekensteyn.nl>
---
 v2: only write optimus_funcs when an Optimus DSM is found.
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 43 ++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index cdf5227..009712a 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 flags_func_detected;
 	acpi_handle dhandle;
 	acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -58,7 +59,6 @@ bool nouveau_is_v1_dsm(void) {
 }
 
 #define NOUVEAU_DSM_HAS_MUX 0x1
-#define NOUVEAU_DSM_HAS_OPT 0x2
 
 #ifdef CONFIG_VGA_SWITCHEROO
 static const char nouveau_dsm_muid[] = {
@@ -110,9 +110,9 @@ 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 uint32_t nouveau_check_optimus_dsm(acpi_handle handle)
 {
-	int result;
+	uint32_t result;
 
 	/*
 	 * Function 0 returns a Buffer containing available functions.
@@ -123,9 +123,13 @@ 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
+	 * If the n-th bit is enabled, function n is supported.
+	 * Check for both bit zero and the NOUVEAU_DSM_OPTIMUS_CAPS since
+	 * some implementations return 0x80000001 on invalid parameters.
 	 */
-	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)
@@ -212,10 +216,11 @@ 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 int nouveau_dsm_pci_probe(struct pci_dev *pdev, uint32_t *optimus_funcs)
 {
 	acpi_handle dhandle;
 	int retval = 0;
+	uint32_t supported_funcs;
 
 	dhandle = ACPI_HANDLE(&pdev->dev);
 	if (!dhandle)
@@ -228,11 +233,10 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
 			   1 << NOUVEAU_DSM_POWER))
 		retval |= NOUVEAU_DSM_HAS_MUX;
 
-	if (nouveau_check_optimus_dsm(dhandle))
-		retval |= NOUVEAU_DSM_HAS_OPT;
-
-	if (retval & NOUVEAU_DSM_HAS_OPT) {
+	supported_funcs = nouveau_check_optimus_dsm(dhandle);
+	if (supported_funcs) {
 		uint32_t result;
+		*optimus_funcs = supported_funcs;
 		nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
 				    &result);
 		dev_info(&pdev->dev, "optimus capabilities: %s, status
%s%s\n",
@@ -252,7 +256,7 @@ static bool nouveau_dsm_detect(void)
 	struct acpi_buffer buffer = {sizeof(acpi_method_name), acpi_method_name};
 	struct pci_dev *pdev = NULL;
 	int has_dsm = 0;
-	int has_optimus = 0;
+	uint32_t optimus_funcs = 0;
 	int vga_count = 0;
 	bool guid_valid;
 	int retval;
@@ -268,30 +272,28 @@ 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);
+		retval = nouveau_dsm_pci_probe(pdev, &optimus_funcs);
 		if (retval & NOUVEAU_DSM_HAS_MUX)
 			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
 	}
 
 	while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL)
{
 		vga_count++;
 
-		retval = nouveau_dsm_pci_probe(pdev);
+		retval = nouveau_dsm_pci_probe(pdev, &optimus_funcs);
 		if (retval & NOUVEAU_DSM_HAS_MUX)
 			has_dsm |= 1;
-		if (retval & NOUVEAU_DSM_HAS_OPT)
-			has_optimus = 1;
 	}
 
 	/* find the optimus DSM or the old v1 DSM */
-	if (has_optimus == 1) {
+	if (optimus_funcs) {
 		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;
+		nouveau_dsm_priv.flags_func_detected +			optimus_funcs & (1 <<
NOUVEAU_DSM_OPTIMUS_FLAGS);
 		ret = true;
 	} else if (vga_count == 2 && has_dsm && guid_valid) {
 		acpi_get_name(nouveau_dsm_priv.dhandle, ACPI_FULL_PATHNAME,
@@ -324,8 +326,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.flags_func_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.8.2