[PATCH 0/3] Apple GMUX hybrid graphics support for MacBookPro15,1

[PATCH 0/3] Apple GMUX hybrid graphics support for MacBookPro15,1

[Andre Eikmeyer]

Hello everyone,

This series will enable hybrid graphics on MacBookPro15,1.

The MacBookPro15,1 normally starts Linux with the discrete GPU as primary.
On the machines tested here that results in roughly 24 W idle power draw,
about 12 W of which is attributable to the otherwise unused discrete GPU.
Selecting the integrated GPU at boot avoids that cost, but the discrete GPU
cannot currently return after GMUX powers it down, so usable hybrid graphics
and suspend are not available.

This series provides the missing power lifecycle across apple-gmux, amdgpu
and the HDA controller. With the integrated GPU as primary, the discrete GPU
then transitions between DynOff and DynPwr and offloading to the dGPU
through DRI_PRIME is working as well as external monitor support.

The result has been tested for three weeks on both the 2018 and 2019
MacBookPro15,1 revisions. Runtime suspend and resume, system suspend and
resume, repeated GPU wakeups and also a variety of external Thunderbolt
and USB-C monitors were tested successfully.
It is a huge step forward in terms of usability, convenience and battery life
for everyone who tested.

So I have this one question regarding Apple's gpu-power-prefs EFI variable.
The firmware defaults to the discrete GPU, while selecting the integrated GPU
is required before this hybrid configuration can take effect. Would setting
that preference from the kernel be appropriate on a known-good model, or
should it remain a userspace policy decision? Some discoverable way to select
the power-efficient configuration would seem preferable to requiring users to
know about a vendor-specific NVRAM variable before Linux behaves like a hybrid
graphics laptop.

Thank you for your time and consideration.

Andre

[PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15, 1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

Hello everyone,

We managed to make hybrid graphics work on the MacBook Pro 15,1 with
iGPU as primary and dGPU with DynOff/DynPwr for offloading using
DRI_PRIME. This needed a few changes in apple-gmux, amdgpu and ALSA.

The discrete GPU on the MacBookPro15,1 did not return with the original
GMUX power-on sequence. The PCI configuration space remained inaccessible
and runtime PM could not provide usable hybrid graphics with the iGPU
as primary.

The firmware PWG1 and PWG3 link methods are evaluated around the GMUX
transition, and power-on completes after PCI configuration space becomes
accessible. The sequence is limited to the MacBookPro15,1 while every
other model is unaffected. Unfortunately, we were not able to make
MacBookPro16,1 and 16,4 dGPUs return from D3cold yet. But we believe
this series will also serve as a good base for upcoming fixes.

This patch was tested on both the 2018 and 2019 MacBookPro15,1 revisions
with the integrated GPU as primary. The discrete GPU transitions between
DynOff and DynPwr, and also external display work across the transitions.
When an external display is connected, the dGPU will turn on automatically
and also turn off again when disconnecting.

Thank you for your time and consideration.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 84 ++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..9154348 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,34 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	unsigned long long result;
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_integer(handle, (acpi_string)method, NULL,
+				       &result);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	if (result) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"%s failed: %llu\n", method, result);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +541,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +615,14 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    !apple_gmux_data->discrete_pdev)
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +872,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1080,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

Hello everyone,

We managed to make hybrid graphics work on the MacBook Pro 15,1 with
iGPU as primary and dGPU with DynOff/DynPwr for offloading using
DRI_PRIME. This needed a few changes in apple-gmux, amdgpu and ALSA.

The discrete GPU on the MacBookPro15,1 did not return with the original
GMUX power-on sequence. The PCI configuration space remained inaccessible
and runtime PM could not provide usable hybrid graphics with the iGPU
as primary.

The firmware PWG1 and PWG3 link methods are evaluated around the GMUX
transition, and power-on completes after PCI configuration space becomes
accessible. The sequence is limited to the MacBookPro15,1 while every
other model is unaffected. Unfortunately, we were not able to make
MacBookPro16,1 and 16,4 dGPUs return from D3cold yet. But we believe
this series will also serve as a good base for upcoming fixes.

This patch was tested on both the 2018 and 2019 MacBookPro15,1 revisions
with the integrated GPU as primary. The discrete GPU transitions between
DynOff and DynPwr, and also external display work across the transitions.
When an external display is connected, the dGPU will turn on automatically
and also turn off again when disconnecting.

Thank you for your time and consideration.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 84 ++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..9154348 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,34 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	unsigned long long result;
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_integer(handle, (acpi_string)method, NULL,
+				       &result);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	if (result) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"%s failed: %llu\n", method, result);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +541,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +615,14 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    !apple_gmux_data->discrete_pdev)
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +872,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1080,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[Andre Eikmeyer]

Hello everyone,

The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
but it does not expose the ATPX or ACPI power-resource interfaces currently
recognized by amdgpu. The driver therefore leaves runtime PM disabled even
though vga_switcheroo can switch the GPU reliably.

A dedicated GMUX runtime PM mode uses the existing vga_switcheroo PM domain
to sequence GPU power. A runtime-suspended GMUX GPU can remain powered off
across system sleep, following the smart-suspend handling already used by
BOCO devices.

This allows the discrete GPU to enter D3cold while the integrated GPU is
primary and allows PRIME workloads and external displays to wake it on
demand. The model enablement is limited to the tested MacBookPro15,1.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions together
with the apple-gmux and HDA changes in this series.

Thank you for your time and consideration.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 30 ++++++++++++-------
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  1 +
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410..cc2e2e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1402,6 +1402,7 @@ int amdgpu_device_mode1_reset(struct amdgpu_device *adev);
 int amdgpu_device_link_reset(struct amdgpu_device *adev);
 bool amdgpu_device_supports_atpx(struct amdgpu_device *adev);
 bool amdgpu_device_supports_px(struct amdgpu_device *adev);
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev);
 bool amdgpu_device_supports_boco(struct amdgpu_device *adev);
 bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev);
 int amdgpu_device_supports_baco(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bff3e06..bfa2b7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/iommu.h>
 #include <linux/pci.h>
+#include <linux/dmi.h>
 #include <linux/pci-p2pdma.h>
 #include <linux/apple-gmux.h>
 #include <linux/nospec.h>
@@ -568,6 +569,22 @@ bool amdgpu_device_supports_px(struct amdgpu_device *adev)
 	return false;
 }
 
+static const struct dmi_system_id amdgpu_gmux_runpm_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro15,1"),
+		},
+	},
+	{ }
+};
+
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev)
+{
+	return dmi_check_system(amdgpu_gmux_runpm_dmi_table) &&
+	       !dev_is_removable(&adev->pdev->dev) &&
+	       apple_gmux_detect(NULL, NULL);
+}
+
 /**
  * amdgpu_device_supports_boco - Is the device a dGPU with ACPI power resources
  *
@@ -631,6 +648,9 @@ void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev)
 			/* enable PX as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
 			dev_info(adev->dev, "Using ATPX for runtime pm\n");
+		} else if (amdgpu_device_supports_gmux(adev)) {
+			adev->pm.rpm_mode = AMDGPU_RUNPM_GMUX;
+			dev_info(adev->dev, "Using Apple GMUX for runtime pm\n");
 		} else if (amdgpu_device_supports_boco(adev)) {
 			/* enable boco as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
@@ -3711,7 +3731,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 {
 	struct pci_dev *pdev = adev->pdev;
 	int r, i;
-	bool px = false;
+	bool gmux, px = false;
 	u32 max_MBps;
 	int tmp;
 
@@ -4144,13 +4164,14 @@ fence_driver_init:
 		vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
-				apple_gmux_detect(NULL, NULL)))
+		   apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_register_client(adev->pdev,
-					       &amdgpu_switcheroo_ops, px);
+					       &amdgpu_switcheroo_ops, px || gmux);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
 
 	adev->pm_nb.notifier_call = amdgpu_device_pm_notifier;
@@ -4278,7 +4299,7 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 {
 	int i, idx;
-	bool px;
+	bool gmux, px;
 
 	amdgpu_device_ip_fini(adev);
 	amdgpu_fence_driver_sw_fini(adev);
@@ -4309,12 +4330,13 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 	adev->xcp_mgr = NULL;
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
 				apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_unregister_client(adev->pdev);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1aed121..b8d4273 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2485,11 +2485,12 @@ retry_init:
 		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
 
 	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
-		/* only need to skip on ATPX */
+		/* ATPX requires a full system-sleep transition. */
 		if (amdgpu_device_supports_px(adev))
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
-		/* we want direct complete for BOCO */
-		if (amdgpu_device_supports_boco(adev))
+		/* BOCO and GMUX can remain runtime suspended across system sleep. */
+		if (amdgpu_device_supports_boco(adev) ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
 						DPM_FLAG_SMART_SUSPEND |
 						DPM_FLAG_MAY_SKIP_RESUME);
@@ -2598,7 +2599,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
-	if (amdgpu_device_supports_boco(adev) && pm_runtime_suspended(dev))
+	if ((amdgpu_device_supports_boco(adev) ||
+	     adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) &&
+	    pm_runtime_suspended(dev))
 		return 1;
 
 	/* if we will not support s3 or s2i for the device
@@ -2860,7 +2863,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 	/*
@@ -2887,8 +2891,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
 		adev->mp1_state = PP_MP1_STATE_NONE;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
-		/* Only need to handle PCI state in the driver for ATPX
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		amdgpu_device_cache_pci_state(pdev);
@@ -2935,10 +2940,11 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!pci_device_is_present(adev->pdev))
 		adev->no_hw_access = true;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		/* Only need to handle PCI state in the driver for ATPX
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		pci_set_power_state(pdev, PCI_D0);
@@ -2958,12 +2964,14 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	}
 	ret = amdgpu_device_resume(drm_dev, false);
 	if (ret) {
-		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			pci_disable_device(pdev);
 		return ret;
 	}
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 
 	amdgpu_restore_umd_profile_pstate_after_runpm(adev);
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 8d1b097..df24b01 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -48,6 +48,7 @@ enum amdgpu_int_thermal_type {
 enum amdgpu_runpm_mode {
 	AMDGPU_RUNPM_NONE,
 	AMDGPU_RUNPM_PX,
+	AMDGPU_RUNPM_GMUX,
 	AMDGPU_RUNPM_BOCO,
 	AMDGPU_RUNPM_BACO,
 	AMDGPU_RUNPM_BAMACO,
-- 
2.55.0

[PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Andre Eikmeyer]

Hello everyone,

The HDA controller normally needs different WAKEEN settings for runtime and
system suspend, so azx_prepare() prevents direct complete. For a switcheroo
audio client whose bound GPU is already runtime suspended and switched off,
however, the controller is physically inaccessible and cannot signal wake
events.

Resuming the HDA function during system suspend also acquires its runtime
PM supplier. This powers the discrete GPU back on only to suspend it again
and can expose failures while restoring an otherwise unused GPU.

Direct complete is now allowed when the bound VGA device is both runtime
suspended and reported off by vga_switcheroo. Active GPUs and HDA
controllers without a switcheroo binding retain the existing WAKEEN
transition.

This was tested as part of the MacBookPro15,1 hybrid graphics series on
both the 2018 and 2019 revisions. The discrete GPU remains powered off
across system suspend when it is unused, and suspend and resume complete
successfully.

Thank you for your time and consideration.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 sound/hda/controllers/intel.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 28c55c5..20ddcc5 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -334,6 +334,7 @@ enum {
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static struct pci_dev *get_bound_vga(struct pci_dev *pci);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
@@ -1016,6 +1017,28 @@ static void __azx_runtime_resume(struct azx *chip)
 		display_power(chip, false);
 }
 
+static bool azx_vga_is_powered_off(struct azx *chip)
+{
+#ifdef SUPPORT_VGA_SWITCHEROO
+	struct pci_dev *pci;
+	bool powered_off = false;
+
+	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
+		return false;
+
+	pci = get_bound_vga(chip->pci);
+	if (pci) {
+		powered_off = pm_runtime_suspended(&pci->dev) &&
+			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
+		pci_dev_put(pci);
+	}
+
+	return powered_off;
+#else
+	return false;
+#endif
+}
+
 static int azx_prepare(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
@@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
 		return 0;
 
 	chip = card->private_data;
+	/* A powered-off dGPU cannot signal HDA wake events. */
+	if (azx_vga_is_powered_off(chip))
+		return 1;
+
 	chip->pm_prepared = 1;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
@@ -1183,8 +1210,6 @@ static const struct dev_pm_ops azx_pm = {
 static int azx_probe_continue(struct azx *chip);
 
 #ifdef SUPPORT_VGA_SWITCHEROO
-static struct pci_dev *get_bound_vga(struct pci_dev *pci);
-
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
-- 
2.55.0

Re: [PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15, 1 dGPU power sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `discrete_pdev` pointer caches the discrete GPU `pci_dev` but is not updated when the client is unregistered and a new device is registered, preventing power switching from functioning if the PCI device is rescanned.
--

--- Patch [1]: [PATCH 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence ---
commit 35744aea76b96c0341cb3a18f76cff6290e2d999
Author: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

This commit introduces a dGPU power sequence for MacBookPro15,1 devices.
It introduces ACPI method evaluation during gmux transitions and verifies
PCI configuration space accessibility before completing power-on.

> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index fbc30f1f8abdf..1771c6bb756ef 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c

[ ... ]

> @@ -549,11 +615,14 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
>  	 */
>  	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
>  		return VGA_SWITCHEROO_IGD;
> -	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> -		 pdev->device == 0x0863)
> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
>  		return VGA_SWITCHEROO_IGD;
> -	else
> -		return VGA_SWITCHEROO_DIS;
> +
> +	if (apple_gmux_data->use_pwg_power_sequence &&
> +	    !apple_gmux_data->discrete_pdev)
> +		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);

[Severity: Medium]
Does this caching mechanism properly handle the PCI device being removed and
rescanned?

If an administrator logically removes the discrete GPU via sysfs (for example,
echo 1 > /sys/bus/pci/devices/.../remove) and rescans the PCI bus, the GPU
driver will re-register with vga_switcheroo.

Because of the !apple_gmux_data->discrete_pdev check, discrete_pdev won't be
updated when the new device is registered. The power switching could break
permanently by returning -ETIMEDOUT, because it will attempt to read the
configuration space on the stale, disconnected pci_dev.

Should there be a way to clear or update the cached discrete_pdev when a
client is unregistered?

> +
> +	return VGA_SWITCHEROO_DIS;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812122206.193680-1-dev@deq.rocks?part=1

Re: [PATCH 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but switch_power_state remains permanently stuck in DRM_SWITCH_POWER_CHANGING on suspend/resume failure.
--

--- Patch [2]: [PATCH 2/3] drm/amdgpu: add Apple GMUX runtime PM support ---
commit 2067ef2b3a267308c80f88d728f87fe7fad945a3
Author: Andre Eikmeyer <dev@deq.rocks>

    drm/amdgpu: add Apple GMUX runtime PM support
    
    The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
    but it does not expose the ATPX or ACPI power-resource interfaces currently
    recognized by amdgpu. The driver therefore leaves runtime PM disabled even
    though vga_switcheroo can switch the GPU reliably.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..a231a002831bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
[ ... ]
> @@ -2872,7 +2875,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>  	}
>  
>  	adev->in_runpm = true;
> -	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> +	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;

[Severity: High]
This is a pre-existing issue, but I noticed that switch_power_state can be
left permanently stuck in DRM_SWITCH_POWER_CHANGING if a failure occurs here.

Looking at amdgpu_pmops_runtime_suspend(), if amdgpu_device_suspend() fails,
it returns early:

	ret = amdgpu_device_suspend(drm_dev, false);
	if (ret) {
		adev->in_runpm = false;
		if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
			adev->mp1_state = PP_MP1_STATE_NONE;
		return ret;
	}

Because switch_power_state is never reset on this error path, drm_open()
will explicitly reject new opens by returning -EINVAL. Does this mean a
failed runtime suspend renders the GPU completely unusable for new clients
until reboot?

[ ... ]
> @@ -2947,10 +2952,11 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>  	if (!pci_device_is_present(adev->pdev))
>  		adev->no_hw_access = true;
>  
> -	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
> +	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
>  		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;

[Severity: High]
This is also a pre-existing issue, but the same state machine corruption can
occur during resume.

If either pci_enable_device() or amdgpu_device_resume() fails, the function
returns without resetting switch_power_state from DRM_SWITCH_POWER_CHANGING:

	ret = amdgpu_device_resume(drm_dev, false);
	if (ret) {
		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
			pci_disable_device(pdev);
		return ret;
	}

Would it be safer to restore switch_power_state on these error paths so
that the state isn't permanently stuck?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812122206.193680-1-dev@deq.rocks?part=2

Re: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Bypassing mandatory suspend preparation in azx_prepare() causes concurrent hardware access and crashes if direct-complete is aborted.
--

--- Patch [3]: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU ---
commit 2f8a96155a50eb983d9e329380eedc7ca760b5b8
Author: Andre Eikmeyer <dev@deq.rocks>

ALSA: hda: allow direct complete with a powered-off GPU

This commit allows direct complete during system suspend for HDA
controllers bound to a powered-off switcheroo GPU. It returns 1
in azx_prepare() if the discrete GPU is runtime suspended and
switched off.

> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 4b03c64e72ab4..47cc695864a11 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c

[ ... ]

> @@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A powered-off dGPU cannot signal HDA wake events. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +

[Severity: High]
Does returning early here bypass setting chip->pm_prepared = 1?

If the PM core aborts direct-complete (for example, if a user enables
wakeup via sysfs, or another device fails to suspend), the fallback
suspend path will execute.

Because chip->pm_prepared is left as 0, when the PCI core triggers a
runtime resume, wouldn't __azx_runtime_resume() incorrectly queue
asynchronous codec resumes via pm_request_resume()?

Could these async resumes execute on the pm_wq workqueue concurrently with
azx_suspend() shutting down the HDA controller, leading to MMIO faults or
system lockups?

>  	chip->pm_prepared = 1;
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812122206.193680-1-dev@deq.rocks?part=3

Re: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Takashi Iwai]

On Wed, 12 Aug 2026 14:22:06 +0200,
Andre Eikmeyer wrote:
> 
> Hello everyone,
> 
> The HDA controller normally needs different WAKEEN settings for runtime and
> system suspend, so azx_prepare() prevents direct complete. For a switcheroo
> audio client whose bound GPU is already runtime suspended and switched off,
> however, the controller is physically inaccessible and cannot signal wake
> events.
> 
> Resuming the HDA function during system suspend also acquires its runtime
> PM supplier. This powers the discrete GPU back on only to suspend it again
> and can expose failures while restoring an otherwise unused GPU.
> 
> Direct complete is now allowed when the bound VGA device is both runtime
> suspended and reported off by vga_switcheroo. Active GPUs and HDA
> controllers without a switcheroo binding retain the existing WAKEEN
> transition.
> 
> This was tested as part of the MacBookPro15,1 hybrid graphics series on
> both the 2018 and 2019 revisions. The discrete GPU remains powered off
> across system suspend when it is unused, and suspend and resume complete
> successfully.
> 
> Thank you for your time and consideration.
> 
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>

Please drop greeting and thank-you texts.  They can be put in the
cover letter, but not in each patch description.

About the code changes:

> +static bool azx_vga_is_powered_off(struct azx *chip)
> +{
> +#ifdef SUPPORT_VGA_SWITCHEROO
> +	struct pci_dev *pci;
> +	bool powered_off = false;
> +
> +	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
> +		return false;
> +
> +	pci = get_bound_vga(chip->pci);
> +	if (pci) {
> +		powered_off = pm_runtime_suspended(&pci->dev) &&
> +			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
> +		pci_dev_put(pci);
> +	}
> +
> +	return powered_off;
> +#else
> +	return false;
> +#endif
> +}

This ifdef can be moved to the caller side below...

> +
>  static int azx_prepare(struct device *dev)
>  {
>  	struct snd_card *card = dev_get_drvdata(dev);
> @@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A powered-off dGPU cannot signal HDA wake events. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +

... to wrap the call site, instead.  Since this is the only caller,
having the conditional here would make the intention clearer.

Also, a comment about what "return 1" plays here would be helpful for
readers.  It's a small piece but has a significant meaning.


thanks,

Takashi

Re: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Takashi Iwai]

On Wed, 12 Aug 2026 15:45:27 +0200,
Takashi Iwai wrote:
> 
> On Wed, 12 Aug 2026 14:22:06 +0200,
> Andre Eikmeyer wrote:
> > 
> > +static bool azx_vga_is_powered_off(struct azx *chip)
> > +{
> > +#ifdef SUPPORT_VGA_SWITCHEROO
> > +	struct pci_dev *pci;
> > +	bool powered_off = false;
> > +
> > +	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
> > +		return false;
> > +
> > +	pci = get_bound_vga(chip->pci);
> > +	if (pci) {
> > +		powered_off = pm_runtime_suspended(&pci->dev) &&
> > +			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
> > +		pci_dev_put(pci);
> > +	}
> > +
> > +	return powered_off;
> > +#else
> > +	return false;
> > +#endif
> > +}
> 
> This ifdef can be moved to the caller side below...
> 
> > +
> >  static int azx_prepare(struct device *dev)
> >  {
> >  	struct snd_card *card = dev_get_drvdata(dev);
> > @@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
> >  		return 0;
> >  
> >  	chip = card->private_data;
> > +	/* A powered-off dGPU cannot signal HDA wake events. */
> > +	if (azx_vga_is_powered_off(chip))
> > +		return 1;
> > +
> 
> ... to wrap the call site, instead.  Since this is the only caller,
> having the conditional here would make the intention clearer.

Looking at the code again, this won't work well.

But, keeping vga_switcheroo-specific code in a single place would be
still meaningful.  So,

 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static bool azx_vga_is_powered_off(struct azx *chip);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
+#define azx_vga_is_powered_off(chip)	false
 #endif

and then define azx_vga_is_powered_off() in the section for
vga_switcheroo code.

In anyway, it's a matter of taste, and if the resultant patch doesn't
look better, you can keep the original way, too.


thanks,

Takashi

[PATCH v2 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Andre Eikmeyer]

The HDA controller normally needs different WAKEEN settings for runtime and
system suspend, so azx_prepare() prevents direct complete. For a switcheroo
audio client whose bound GPU is already runtime suspended and switched off,
however, the controller is physically inaccessible and cannot signal wake
events.

Resuming the HDA function during system suspend also acquires its runtime
PM supplier. This powers the discrete GPU back on only to suspend it again
and can expose failures while restoring an otherwise unused GPU.

Direct complete is now allowed when the bound VGA device is both runtime
suspended and reported off by vga_switcheroo. Active GPUs and HDA
controllers without a switcheroo binding retain the existing WAKEEN
transition.

This was tested as part of the MacBookPro15,1 hybrid graphics series on
both the 2018 and 2019 revisions. The discrete GPU remains powered off
across system suspend when it is unused, and suspend and resume complete
successfully.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
Changes in v2:
- Move the SUPPORT_VGA_SWITCHEROO guard to the sole call site.
- Explain the significance of the positive azx_prepare() return value.

 sound/hda/controllers/intel.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 28c55c5..9132907 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -334,6 +334,7 @@ enum {
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static struct pci_dev *get_bound_vga(struct pci_dev *pci);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
@@ -1016,6 +1017,26 @@ static void __azx_runtime_resume(struct azx *chip)
 		display_power(chip, false);
 }
 
+#ifdef SUPPORT_VGA_SWITCHEROO
+static bool azx_vga_is_powered_off(struct azx *chip)
+{
+	struct pci_dev *pci;
+	bool powered_off = false;
+
+	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
+		return false;
+
+	pci = get_bound_vga(chip->pci);
+	if (pci) {
+		powered_off = pm_runtime_suspended(&pci->dev) &&
+			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
+		pci_dev_put(pci);
+	}
+
+	return powered_off;
+}
+#endif
+
 static int azx_prepare(struct device *dev)
 {
 	struct snd_card *card = dev_get_drvdata(dev);
@@ -1025,6 +1046,12 @@ static int azx_prepare(struct device *dev)
 		return 0;
 
 	chip = card->private_data;
+#ifdef SUPPORT_VGA_SWITCHEROO
+	/* A positive return allows the PM core to use direct complete. */
+	if (azx_vga_is_powered_off(chip))
+		return 1;
+#endif
+
 	chip->pm_prepared = 1;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
@@ -1183,8 +1210,6 @@ static const struct dev_pm_ops azx_pm = {
 static int azx_probe_continue(struct azx *chip);
 
 #ifdef SUPPORT_VGA_SWITCHEROO
-static struct pci_dev *get_bound_vga(struct pci_dev *pci);
-
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
-- 
2.55.0

Re: [PATCH 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Christian König]

On 8/12/26 14:22, Andre Eikmeyer wrote:
> Hello everyone,
> 
> The HDA controller normally needs different WAKEEN settings for runtime and
> system suspend, so azx_prepare() prevents direct complete. For a switcheroo
> audio client whose bound GPU is already runtime suspended and switched off,
> however, the controller is physically inaccessible and cannot signal wake
> events.
> 
> Resuming the HDA function during system suspend also acquires its runtime
> PM supplier. This powers the discrete GPU back on only to suspend it again
> and can expose failures while restoring an otherwise unused GPU.
> 
> Direct complete is now allowed when the bound VGA device is both runtime
> suspended and reported off by vga_switcheroo. Active GPUs and HDA
> controllers without a switcheroo binding retain the existing WAKEEN
> transition.
> 
> This was tested as part of the MacBookPro15,1 hybrid graphics series on
> both the 2018 and 2019 revisions. The discrete GPU remains powered off
> across system suspend when it is unused, and suspend and resume complete
> successfully.
> 
> Thank you for your time and consideration.

That sounds reasonable to me, but I'm definitely not an expert for that kind of stuff.

Adding Harry and Leo from our display team on CC as well since that HDA audio stuff falls into their responsibilities as well, please make sure to CC them on future revisions of this patch set.

I'm not sure that even applies to AMD GPUs but if I strongly suggest to run this patch through our display CI, just to make sure that there is no regression on suspend/resume.

Thanks,
Christian.

> 
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
> ---
>  sound/hda/controllers/intel.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 28c55c5..20ddcc5 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c
> @@ -334,6 +334,7 @@ enum {
>  #ifdef SUPPORT_VGA_SWITCHEROO
>  #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
>  #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
> +static struct pci_dev *get_bound_vga(struct pci_dev *pci);
>  #else
>  #define use_vga_switcheroo(chip)	0
>  #define needs_eld_notify_link(chip)	false
> @@ -1016,6 +1017,28 @@ static void __azx_runtime_resume(struct azx *chip)
>  		display_power(chip, false);
>  }
>  
> +static bool azx_vga_is_powered_off(struct azx *chip)
> +{
> +#ifdef SUPPORT_VGA_SWITCHEROO
> +	struct pci_dev *pci;
> +	bool powered_off = false;
> +
> +	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
> +		return false;
> +
> +	pci = get_bound_vga(chip->pci);
> +	if (pci) {
> +		powered_off = pm_runtime_suspended(&pci->dev) &&
> +			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
> +		pci_dev_put(pci);
> +	}
> +
> +	return powered_off;
> +#else
> +	return false;
> +#endif
> +}
> +
>  static int azx_prepare(struct device *dev)
>  {
>  	struct snd_card *card = dev_get_drvdata(dev);
> @@ -1025,6 +1048,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A powered-off dGPU cannot signal HDA wake events. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +
>  	chip->pm_prepared = 1;
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
>  
> @@ -1183,8 +1210,6 @@ static const struct dev_pm_ops azx_pm = {
>  static int azx_probe_continue(struct azx *chip);
>  
>  #ifdef SUPPORT_VGA_SWITCHEROO
> -static struct pci_dev *get_bound_vga(struct pci_dev *pci);
> -
>  static void azx_vs_set_state(struct pci_dev *pci,
>  			     enum vga_switcheroo_state state)
>  {

[PATCH v3 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Andre Eikmeyer]

The HDA controller normally needs different WAKEEN settings for runtime and
system suspend, so azx_prepare() prevents direct complete. For a switcheroo
audio client whose bound GPU is already runtime suspended and switched off,
however, the controller is physically inaccessible and cannot signal wake
events.

Resuming the HDA function during system suspend also acquires its runtime
PM supplier. This powers the discrete GPU back on only to suspend it again
and can expose failures while restoring an otherwise unused GPU.

Direct complete is now allowed when the bound VGA device is both runtime
suspended and reported off by vga_switcheroo. Active GPUs and HDA
controllers without a switcheroo binding retain the existing WAKEEN
transition.

This was tested as part of the MacBookPro15,1 hybrid graphics series on
both the 2018 and 2019 revisions. The discrete GPU remains powered off
across system suspend when it is unused, and suspend and resume complete
successfully.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
Changes in v3:
- Rework the conditional compilation as suggested in the follow-up review.
- Keep the switcheroo implementation in the existing switcheroo section.

The follow-up review crossed with the v2 submission.

Changes in v2:
- Explain the significance of the positive azx_prepare() return value.

 sound/hda/controllers/intel.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 28c55c5..194d962 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -334,9 +334,11 @@ enum {
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static bool azx_vga_is_powered_off(struct azx *chip);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
+#define azx_vga_is_powered_off(chip)	false
 #endif
 
 static const char * const driver_short_names[] = {
@@ -1025,6 +1027,10 @@ static int azx_prepare(struct device *dev)
 		return 0;
 
 	chip = card->private_data;
+	/* A positive return allows the PM core to use direct complete. */
+	if (azx_vga_is_powered_off(chip))
+		return 1;
+
 	chip->pm_prepared = 1;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
@@ -1185,6 +1191,24 @@ static int azx_probe_continue(struct azx *chip);
 #ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev *get_bound_vga(struct pci_dev *pci);
 
+static bool azx_vga_is_powered_off(struct azx *chip)
+{
+	struct pci_dev *pci;
+	bool powered_off = false;
+
+	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
+		return false;
+
+	pci = get_bound_vga(chip->pci);
+	if (pci) {
+		powered_off = pm_runtime_suspended(&pci->dev) &&
+			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
+		pci_dev_put(pci);
+	}
+
+	return powered_off;
+}
+
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
-- 
2.55.0

Re: [PATCH v3 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Takashi Iwai]

On Wed, 12 Aug 2026 16:23:00 +0200,
Andre Eikmeyer wrote:
> 
> The HDA controller normally needs different WAKEEN settings for runtime and
> system suspend, so azx_prepare() prevents direct complete. For a switcheroo
> audio client whose bound GPU is already runtime suspended and switched off,
> however, the controller is physically inaccessible and cannot signal wake
> events.
> 
> Resuming the HDA function during system suspend also acquires its runtime
> PM supplier. This powers the discrete GPU back on only to suspend it again
> and can expose failures while restoring an otherwise unused GPU.
> 
> Direct complete is now allowed when the bound VGA device is both runtime
> suspended and reported off by vga_switcheroo. Active GPUs and HDA
> controllers without a switcheroo binding retain the existing WAKEEN
> transition.
> 
> This was tested as part of the MacBookPro15,1 hybrid graphics series on
> both the 2018 and 2019 revisions. The discrete GPU remains powered off
> across system suspend when it is unused, and suspend and resume complete
> successfully.
> 
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
> ---
> Changes in v3:
> - Rework the conditional compilation as suggested in the follow-up review.
> - Keep the switcheroo implementation in the existing switcheroo section.
> 
> The follow-up review crossed with the v2 submission.
> 
> Changes in v2:
> - Explain the significance of the positive azx_prepare() return value.

Feel free to take my ack:

Reviewed-by: Takashi Iwai <tiwai@suse.de>

But I guess you'd need to refresh the patch descriptions in other two
patches in the series, so better to resubmit the whole series with
corrected patches :)


thanks,

Takashi

[PATCH v2 0/3] Apple GMUX hybrid graphics support for MacBookPro15,1

[Andre Eikmeyer]

Hello everyone,

This series enables hybrid graphics on the MacBookPro15,1.

The MacBookPro15,1 normally starts Linux with the discrete GPU as primary.
On the machines tested here that results in roughly 24 W idle power draw,
about 12 W of which is attributable to the otherwise unused discrete GPU.
Selecting the integrated GPU at boot avoids that cost, but the discrete GPU
cannot currently return after GMUX powers it down, so usable hybrid graphics
and suspend are not available.

This series provides the missing power lifecycle across apple-gmux, amdgpu
and the HDA controller. With the integrated GPU as primary, the discrete GPU
transitions between DynOff and DynPwr. Offloading through DRI_PRIME and
external monitor support both work across these transitions.

The result has been tested for three weeks on both the 2018 and 2019
MacBookPro15,1 revisions. Runtime suspend and resume, system suspend and
resume, repeated GPU wakeups, and a variety of external Thunderbolt and
USB-C monitors were tested successfully.

One question remains regarding Apple's gpu-power-prefs EFI variable. The
firmware defaults to the discrete GPU, while selecting the integrated GPU
is required before this hybrid configuration can take effect. Would setting
that preference from the kernel be appropriate on a known-good model, or
should it remain a userspace policy decision? Some discoverable way to
select the power-efficient configuration would seem preferable to requiring
users to know about a vendor-specific NVRAM variable.

Changes in v2:
- Refresh the individual patch descriptions.
- Evaluate PWG methods without expecting an integer return object.
- Refresh the cached discrete PCI device when a client is re-registered.
- Keep the HDA switcheroo implementation in the existing switcheroo section.
- Document the positive azx_prepare() return and add Takashi's Reviewed-by.

The HDA follow-up review crossed with the standalone v2 and v3 revisions of
patch 3/3. This full-series v2 contains the reviewed v3 implementation.

v1: https://lore.kernel.org/all/20260812122206.193680-1-dev@deq.rocks/

Thank you for your time and consideration.

Andre Eikmeyer (2):
  drm/amdgpu: add Apple GMUX runtime PM support
  ALSA: hda: allow direct complete with a powered-off GPU

Atharva Tiwari (1):
  platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 30 ++++++---
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  1 +
 drivers/platform/x86/apple-gmux.c          | 78 ++++++++++++++++++++--
 sound/hda/controllers/intel.c              | 24 +++++++
 6 files changed, 145 insertions(+), 23 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

The discrete GPU on the MacBookPro15,1 does not return after the legacy
GMUX power-on sequence. PCI configuration space remains inaccessible, so
runtime PM cannot provide usable hybrid graphics with the integrated GPU
as primary.

Evaluate the firmware PWG1 and PWG3 link methods around the GMUX
transition and wait for PCI configuration space before completing power-on.
Keep the sequence limited to the MacBookPro15,1 and retain the existing
path for every other model.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions with the
integrated GPU as primary. The discrete GPU transitions between DynOff and
DynPwr, and external displays work across the transitions.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 78 ++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..16ccf15 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,26 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_object(handle, (acpi_string)method, NULL, NULL);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +533,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +607,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    apple_gmux_data->discrete_pdev != pdev) {
+		pci_dev_put(apple_gmux_data->discrete_pdev);
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+	}
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +866,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1074,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH v2 1/3] platform/x86: apple-gmux: add MacBookPro15, 1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

The discrete GPU on the MacBookPro15,1 does not return after the legacy
GMUX power-on sequence. PCI configuration space remains inaccessible, so
runtime PM cannot provide usable hybrid graphics with the integrated GPU
as primary.

Evaluate the firmware PWG1 and PWG3 link methods around the GMUX
transition and wait for PCI configuration space before completing power-on.
Keep the sequence limited to the MacBookPro15,1 and retain the existing
path for every other model.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions with the
integrated GPU as primary. The discrete GPU transitions between DynOff and
DynPwr, and external displays work across the transitions.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 78 ++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..16ccf15 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,26 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_object(handle, (acpi_string)method, NULL, NULL);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +533,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +607,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    apple_gmux_data->discrete_pdev != pdev) {
+		pci_dev_put(apple_gmux_data->discrete_pdev);
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+	}
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +866,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1074,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH v2 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[Andre Eikmeyer]

The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
but it does not expose the ATPX or ACPI power-resource interfaces currently
recognized by amdgpu. The driver therefore leaves runtime PM disabled even
though vga_switcheroo can switch the GPU reliably.

A dedicated GMUX runtime PM mode uses the existing vga_switcheroo PM domain
to sequence GPU power. A runtime-suspended GMUX GPU can remain powered off
across system sleep, following the smart-suspend handling already used by
BOCO devices.

This allows the discrete GPU to enter D3cold while the integrated GPU is
primary and allows PRIME workloads and external displays to wake it on
demand. The model enablement is limited to the tested MacBookPro15,1.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions together
with the apple-gmux and HDA changes in this series.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 30 ++++++++++++-------
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  1 +
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410..cc2e2e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1402,6 +1402,7 @@ int amdgpu_device_mode1_reset(struct amdgpu_device *adev);
 int amdgpu_device_link_reset(struct amdgpu_device *adev);
 bool amdgpu_device_supports_atpx(struct amdgpu_device *adev);
 bool amdgpu_device_supports_px(struct amdgpu_device *adev);
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev);
 bool amdgpu_device_supports_boco(struct amdgpu_device *adev);
 bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev);
 int amdgpu_device_supports_baco(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bff3e06..bfa2b7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/iommu.h>
 #include <linux/pci.h>
+#include <linux/dmi.h>
 #include <linux/pci-p2pdma.h>
 #include <linux/apple-gmux.h>
 #include <linux/nospec.h>
@@ -568,6 +569,22 @@ bool amdgpu_device_supports_px(struct amdgpu_device *adev)
 	return false;
 }
 
+static const struct dmi_system_id amdgpu_gmux_runpm_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro15,1"),
+		},
+	},
+	{ }
+};
+
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev)
+{
+	return dmi_check_system(amdgpu_gmux_runpm_dmi_table) &&
+	       !dev_is_removable(&adev->pdev->dev) &&
+	       apple_gmux_detect(NULL, NULL);
+}
+
 /**
  * amdgpu_device_supports_boco - Is the device a dGPU with ACPI power resources
  *
@@ -631,6 +648,9 @@ void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev)
 			/* enable PX as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
 			dev_info(adev->dev, "Using ATPX for runtime pm\n");
+		} else if (amdgpu_device_supports_gmux(adev)) {
+			adev->pm.rpm_mode = AMDGPU_RUNPM_GMUX;
+			dev_info(adev->dev, "Using Apple GMUX for runtime pm\n");
 		} else if (amdgpu_device_supports_boco(adev)) {
 			/* enable boco as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
@@ -3711,7 +3731,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 {
 	struct pci_dev *pdev = adev->pdev;
 	int r, i;
-	bool px = false;
+	bool gmux, px = false;
 	u32 max_MBps;
 	int tmp;
 
@@ -4144,13 +4164,14 @@ fence_driver_init:
 		vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
-				apple_gmux_detect(NULL, NULL)))
+		   apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_register_client(adev->pdev,
-					       &amdgpu_switcheroo_ops, px);
+					       &amdgpu_switcheroo_ops, px || gmux);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
 
 	adev->pm_nb.notifier_call = amdgpu_device_pm_notifier;
@@ -4278,7 +4299,7 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 {
 	int i, idx;
-	bool px;
+	bool gmux, px;
 
 	amdgpu_device_ip_fini(adev);
 	amdgpu_fence_driver_sw_fini(adev);
@@ -4309,12 +4330,13 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 	adev->xcp_mgr = NULL;
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
 				apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_unregister_client(adev->pdev);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1aed121..b8d4273 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2485,11 +2485,12 @@ retry_init:
 		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
 
 	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
-		/* only need to skip on ATPX */
+		/* ATPX requires a full system-sleep transition. */
 		if (amdgpu_device_supports_px(adev))
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
-		/* we want direct complete for BOCO */
-		if (amdgpu_device_supports_boco(adev))
+		/* BOCO and GMUX can remain runtime suspended across system sleep. */
+		if (amdgpu_device_supports_boco(adev) ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
 						DPM_FLAG_SMART_SUSPEND |
 						DPM_FLAG_MAY_SKIP_RESUME);
@@ -2598,7 +2599,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
-	if (amdgpu_device_supports_boco(adev) && pm_runtime_suspended(dev))
+	if ((amdgpu_device_supports_boco(adev) ||
+	     adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) &&
+	    pm_runtime_suspended(dev))
 		return 1;
 
 	/* if we will not support s3 or s2i for the device
@@ -2860,7 +2863,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 	/*
@@ -2887,8 +2891,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
 		adev->mp1_state = PP_MP1_STATE_NONE;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
-		/* Only need to handle PCI state in the driver for ATPX
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		amdgpu_device_cache_pci_state(pdev);
@@ -2935,10 +2940,11 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!pci_device_is_present(adev->pdev))
 		adev->no_hw_access = true;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		/* Only need to handle PCI state in the driver for ATPX
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		pci_set_power_state(pdev, PCI_D0);
@@ -2958,12 +2964,14 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	}
 	ret = amdgpu_device_resume(drm_dev, false);
 	if (ret) {
-		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			pci_disable_device(pdev);
 		return ret;
 	}
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 
 	amdgpu_restore_umd_profile_pstate_after_runpm(adev);
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 8d1b097..df24b01 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -48,6 +48,7 @@ enum amdgpu_int_thermal_type {
 enum amdgpu_runpm_mode {
 	AMDGPU_RUNPM_NONE,
 	AMDGPU_RUNPM_PX,
+	AMDGPU_RUNPM_GMUX,
 	AMDGPU_RUNPM_BOCO,
 	AMDGPU_RUNPM_BACO,
 	AMDGPU_RUNPM_BAMACO,
-- 
2.55.0

[PATCH v2 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Andre Eikmeyer]

The HDA controller normally needs different WAKEEN settings for runtime and
system suspend, so azx_prepare() prevents direct complete. For a switcheroo
audio client whose bound GPU is already runtime suspended and switched off,
however, the controller is physically inaccessible and cannot signal wake
events.

Resuming the HDA function during system suspend also acquires its runtime
PM supplier. This powers the discrete GPU back on only to suspend it again
and can expose failures while restoring an otherwise unused GPU.

Direct complete is allowed when the bound VGA device is both runtime
suspended and reported off by vga_switcheroo. Active GPUs and HDA
controllers without a switcheroo binding retain the existing WAKEEN
transition.

This was tested as part of the MacBookPro15,1 hybrid graphics series on
both the 2018 and 2019 revisions. The discrete GPU remains powered off
across system suspend when it is unused, and suspend and resume complete
successfully.

Reviewed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 sound/hda/controllers/intel.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 28c55c5..194d962 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -334,9 +334,11 @@ enum {
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static bool azx_vga_is_powered_off(struct azx *chip);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
+#define azx_vga_is_powered_off(chip)	false
 #endif
 
 static const char * const driver_short_names[] = {
@@ -1025,6 +1027,10 @@ static int azx_prepare(struct device *dev)
 		return 0;
 
 	chip = card->private_data;
+	/* A positive return allows the PM core to use direct complete. */
+	if (azx_vga_is_powered_off(chip))
+		return 1;
+
 	chip->pm_prepared = 1;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
@@ -1185,6 +1191,24 @@ static int azx_probe_continue(struct azx *chip);
 #ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev *get_bound_vga(struct pci_dev *pci);
 
+static bool azx_vga_is_powered_off(struct azx *chip)
+{
+	struct pci_dev *pci;
+	bool powered_off = false;
+
+	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
+		return false;
+
+	pci = get_bound_vga(chip->pci);
+	if (pci) {
+		powered_off = pm_runtime_suspended(&pci->dev) &&
+			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
+		pci_dev_put(pci);
+	}
+
+	return powered_off;
+}
+
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
-- 
2.55.0

[PATCH v3 0/3] Apple GMUX hybrid graphics support for MacBookPro15,1

[Andre Eikmeyer]

Hello everyone,

This series enables hybrid graphics on the MacBookPro15,1.

The MacBookPro15,1 normally starts Linux with the discrete GPU as primary.
On the machines tested here that results in roughly 24 W idle power draw,
about 12 W of which is attributable to the otherwise unused discrete GPU.
Selecting the integrated GPU at boot avoids that cost, but the discrete GPU
cannot currently return after GMUX powers it down, so usable hybrid graphics
and suspend are not available.

This series provides the missing power lifecycle across apple-gmux, amdgpu
and the HDA controller. With the integrated GPU as primary, the discrete GPU
transitions between DynOff and DynPwr. Offloading through DRI_PRIME and
external monitor support both work across these transitions.

The result has been tested for three weeks on both the 2018 and 2019
MacBookPro15,1 revisions. Runtime suspend and resume, system suspend and
resume, repeated GPU wakeups, and a variety of external Thunderbolt and
USB-C monitors were tested successfully.

One question remains regarding Apple's gpu-power-prefs EFI variable. The
firmware defaults to the discrete GPU, while selecting the integrated GPU
is required before this hybrid configuration can take effect. Would setting
that preference from the kernel be appropriate on a known-good model, or
should it remain a userspace policy decision? Some discoverable way to
select the power-efficient configuration would seem preferable to requiring
users to know about a vendor-specific NVRAM variable.

Changes in v3:
- Restore integer result validation for PWG1 and PWG3. The MacBookPro15,1
  AML returns Zero on success and One on failure from both methods. The v2
  conversion to acpi_evaluate_object() incorrectly discarded that status.

Changes in v2:
- Refresh the individual patch descriptions.
- Refresh the cached discrete PCI device when a client is re-registered.
- Keep the HDA switcheroo implementation in the existing switcheroo section.
- Document the positive azx_prepare() return and add Takashi's Reviewed-by.

v2: https://lore.kernel.org/all/20260812144221.35877-1-dev@deq.rocks/
v1: https://lore.kernel.org/all/20260812122206.193680-1-dev@deq.rocks/

Thank you for your time and consideration.

Andre Eikmeyer (2):
  drm/amdgpu: add Apple GMUX runtime PM support
  ALSA: hda: allow direct complete with a powered-off GPU

Atharva Tiwari (1):
  platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 30 +++++---
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  1 +
 drivers/platform/x86/apple-gmux.c          | 86 ++++++++++++++++++++--
 sound/hda/controllers/intel.c              | 24 ++++++
 6 files changed, 153 insertions(+), 23 deletions(-)

-- 
2.55.0

[PATCH v3 1/3] platform/x86: apple-gmux: add MacBookPro15, 1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

The discrete GPU on the MacBookPro15,1 does not return after the legacy
GMUX power-on sequence. PCI configuration space remains inaccessible, so
runtime PM cannot provide usable hybrid graphics with the integrated GPU
as primary.

Evaluate the firmware PWG1 and PWG3 link methods around the GMUX
transition and wait for PCI configuration space before completing power-on.
Keep the sequence limited to the MacBookPro15,1 and retain the existing
path for every other model.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions with the
integrated GPU as primary. The discrete GPU transitions between DynOff and
DynPwr, and external displays work across the transitions.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 86 ++++++++++++++++++++++++++++---
 1 file changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..ccb059a 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,34 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	unsigned long long result;
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_integer(handle, (acpi_string)method, NULL,
+				       &result);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	if (result) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"%s failed: %llu\n", method, result);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +541,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +615,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    apple_gmux_data->discrete_pdev != pdev) {
+		pci_dev_put(apple_gmux_data->discrete_pdev);
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+	}
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +874,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1082,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH v3 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

[Andre Eikmeyer]

From: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

The discrete GPU on the MacBookPro15,1 does not return after the legacy
GMUX power-on sequence. PCI configuration space remains inaccessible, so
runtime PM cannot provide usable hybrid graphics with the integrated GPU
as primary.

Evaluate the firmware PWG1 and PWG3 link methods around the GMUX
transition and wait for PCI configuration space before completing power-on.
Keep the sequence limited to the MacBookPro15,1 and retain the existing
path for every other model.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions with the
integrated GPU as primary. The discrete GPU transitions between DynOff and
DynPwr, and external displays work across the transitions.

Co-developed-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 86 ++++++++++++++++++++++++++++---
 1 file changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9c728ac..ccb059a 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
 #include <linux/debugfs.h>
+#include <linux/dmi.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
@@ -74,6 +75,8 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+	struct pci_dev *discrete_pdev;
+	bool use_pwg_power_sequence;
 
 	/* debugfs data */
 	u8 selected_port;
@@ -82,6 +85,34 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+static int gmux_call_pwg(struct apple_gmux_data *gmux_data,
+			 const char *method)
+{
+	acpi_handle handle = ACPI_HANDLE(&gmux_data->discrete_pdev->dev);
+	unsigned long long result;
+	acpi_status status;
+
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_integer(handle, (acpi_string)method, NULL,
+				       &result);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"failed to evaluate %s: %s\n", method,
+			acpi_format_exception(status));
+		return -EIO;
+	}
+
+	if (result) {
+		dev_err(&gmux_data->discrete_pdev->dev,
+			"%s failed: %llu\n", method, result);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 struct apple_gmux_config {
 	u8 (*read8)(struct apple_gmux_data *gmux_data, int port);
 	void (*write8)(struct apple_gmux_data *gmux_data, int port, u8 val);
@@ -510,14 +541,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
 				   enum vga_switcheroo_state state)
 {
+	int ret;
+
 	reinit_completion(&gmux_data->powerchange_done);
 
 	if (state == VGA_SWITCHEROO_ON) {
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
-		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		if (gmux_data->use_pwg_power_sequence &&
+		    gmux_data->discrete_pdev) {
+			u16 vendor;
+			int i;
+
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 2);
+			msleep(100);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+
+			ret = gmux_call_pwg(gmux_data, "PWG1");
+			if (ret)
+				return ret;
+
+			for (i = 0; i < 1000; i++) {
+				pci_read_config_word(gmux_data->discrete_pdev,
+						     PCI_VENDOR_ID, &vendor);
+				if (vendor != 0xffff)
+					break;
+				usleep_range(1000, 2000);
+			}
+			if (vendor == 0xffff) {
+				dev_err(&gmux_data->discrete_pdev->dev,
+					"timed out waiting for PCI config space\n");
+				return -ETIMEDOUT;
+			}
+
+			ret = gmux_call_pwg(gmux_data, "PWG3");
+			if (ret)
+				return ret;
+		} else {
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+			gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
+		}
 		pr_debug("Discrete card powered up\n");
 	} else {
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
+		if (gmux_data->use_pwg_power_sequence)
+			usleep_range(10000, 11000);
 		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 0);
 		pr_debug("Discrete card powered down\n");
 	}
@@ -549,11 +615,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 	 */
 	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
 		return VGA_SWITCHEROO_IGD;
-	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
-		 pdev->device == 0x0863)
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
 		return VGA_SWITCHEROO_IGD;
-	else
-		return VGA_SWITCHEROO_DIS;
+
+	if (apple_gmux_data->use_pwg_power_sequence &&
+	    apple_gmux_data->discrete_pdev != pdev) {
+		pci_dev_put(apple_gmux_data->discrete_pdev);
+		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
+	}
+
+	return VGA_SWITCHEROO_DIS;
 }
 
 static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
@@ -803,6 +874,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	if (!gmux_data)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
+	gmux_data->use_pwg_power_sequence = type == APPLE_GMUX_TYPE_MMIO &&
+		dmi_match(DMI_PRODUCT_NAME, "MacBookPro15,1");
 
 	switch (type) {
 	case APPLE_GMUX_TYPE_MMIO:
@@ -1009,6 +1082,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 	} else
 		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
+	pci_dev_put(gmux_data->discrete_pdev);
 	kfree(gmux_data);
 }
 
-- 
2.55.0

[PATCH v3 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[Andre Eikmeyer]

The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
but it does not expose the ATPX or ACPI power-resource interfaces currently
recognized by amdgpu. The driver therefore leaves runtime PM disabled even
though vga_switcheroo can switch the GPU reliably.

A dedicated GMUX runtime PM mode uses the existing vga_switcheroo PM domain
to sequence GPU power. A runtime-suspended GMUX GPU can remain powered off
across system sleep, following the smart-suspend handling already used by
BOCO devices.

This allows the discrete GPU to enter D3cold while the integrated GPU is
primary and allows PRIME workloads and external displays to wake it on
demand. The model enablement is limited to the tested MacBookPro15,1.

This was tested on both the 2018 and 2019 MacBookPro15,1 revisions together
with the apple-gmux and HDA changes in this series.

Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 ++++++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 30 ++++++++++++-------
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  1 +
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410..cc2e2e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1402,6 +1402,7 @@ int amdgpu_device_mode1_reset(struct amdgpu_device *adev);
 int amdgpu_device_link_reset(struct amdgpu_device *adev);
 bool amdgpu_device_supports_atpx(struct amdgpu_device *adev);
 bool amdgpu_device_supports_px(struct amdgpu_device *adev);
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev);
 bool amdgpu_device_supports_boco(struct amdgpu_device *adev);
 bool amdgpu_device_supports_smart_shift(struct amdgpu_device *adev);
 int amdgpu_device_supports_baco(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bff3e06..bfa2b7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/iommu.h>
 #include <linux/pci.h>
+#include <linux/dmi.h>
 #include <linux/pci-p2pdma.h>
 #include <linux/apple-gmux.h>
 #include <linux/nospec.h>
@@ -568,6 +569,22 @@ bool amdgpu_device_supports_px(struct amdgpu_device *adev)
 	return false;
 }
 
+static const struct dmi_system_id amdgpu_gmux_runpm_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro15,1"),
+		},
+	},
+	{ }
+};
+
+bool amdgpu_device_supports_gmux(struct amdgpu_device *adev)
+{
+	return dmi_check_system(amdgpu_gmux_runpm_dmi_table) &&
+	       !dev_is_removable(&adev->pdev->dev) &&
+	       apple_gmux_detect(NULL, NULL);
+}
+
 /**
  * amdgpu_device_supports_boco - Is the device a dGPU with ACPI power resources
  *
@@ -631,6 +648,9 @@ void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev)
 			/* enable PX as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
 			dev_info(adev->dev, "Using ATPX for runtime pm\n");
+		} else if (amdgpu_device_supports_gmux(adev)) {
+			adev->pm.rpm_mode = AMDGPU_RUNPM_GMUX;
+			dev_info(adev->dev, "Using Apple GMUX for runtime pm\n");
 		} else if (amdgpu_device_supports_boco(adev)) {
 			/* enable boco as runtime mode */
 			adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
@@ -3711,7 +3731,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 {
 	struct pci_dev *pdev = adev->pdev;
 	int r, i;
-	bool px = false;
+	bool gmux, px = false;
 	u32 max_MBps;
 	int tmp;
 
@@ -4144,13 +4164,14 @@ fence_driver_init:
 		vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
-				apple_gmux_detect(NULL, NULL)))
+		   apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_register_client(adev->pdev,
-					       &amdgpu_switcheroo_ops, px);
+					       &amdgpu_switcheroo_ops, px || gmux);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
 
 	adev->pm_nb.notifier_call = amdgpu_device_pm_notifier;
@@ -4278,7 +4299,7 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
 void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 {
 	int i, idx;
-	bool px;
+	bool gmux, px;
 
 	amdgpu_device_ip_fini(adev);
 	amdgpu_fence_driver_sw_fini(adev);
@@ -4309,12 +4330,13 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
 	adev->xcp_mgr = NULL;
 
 	px = amdgpu_device_supports_px(adev);
+	gmux = amdgpu_device_supports_gmux(adev);
 
 	if (px || (!dev_is_removable(&adev->pdev->dev) &&
 				apple_gmux_detect(NULL, NULL)))
 		vga_switcheroo_unregister_client(adev->pdev);
 
-	if (px)
+	if (px || gmux)
 		vga_switcheroo_fini_domain_pm_ops(adev->dev);
 
 	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 1aed121..b8d4273 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2485,11 +2485,12 @@ retry_init:
 		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
 
 	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
-		/* only need to skip on ATPX */
+		/* ATPX requires a full system-sleep transition. */
 		if (amdgpu_device_supports_px(adev))
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
-		/* we want direct complete for BOCO */
-		if (amdgpu_device_supports_boco(adev))
+		/* BOCO and GMUX can remain runtime suspended across system sleep. */
+		if (amdgpu_device_supports_boco(adev) ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
 						DPM_FLAG_SMART_SUSPEND |
 						DPM_FLAG_MAY_SKIP_RESUME);
@@ -2598,7 +2599,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
 	/* Return a positive number here so
 	 * DPM_FLAG_SMART_SUSPEND works properly
 	 */
-	if (amdgpu_device_supports_boco(adev) && pm_runtime_suspended(dev))
+	if ((amdgpu_device_supports_boco(adev) ||
+	     adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) &&
+	    pm_runtime_suspended(dev))
 		return 1;
 
 	/* if we will not support s3 or s2i for the device
@@ -2860,7 +2863,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	}
 
 	adev->in_runpm = true;
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 	/*
@@ -2887,8 +2891,9 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
 		adev->mp1_state = PP_MP1_STATE_NONE;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
-		/* Only need to handle PCI state in the driver for ATPX
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		amdgpu_device_cache_pci_state(pdev);
@@ -2935,10 +2940,11 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	if (!pci_device_is_present(adev->pdev))
 		adev->no_hw_access = true;
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		/* Only need to handle PCI state in the driver for ATPX
+		/* Only need to handle PCI state in the driver for ATPX and GMUX.
 		 * PCI core handles it for _PR3.
 		 */
 		pci_set_power_state(pdev, PCI_D0);
@@ -2958,12 +2964,14 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
 	}
 	ret = amdgpu_device_resume(drm_dev, false);
 	if (ret) {
-		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 			pci_disable_device(pdev);
 		return ret;
 	}
 
-	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
+	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
+	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
 
 	amdgpu_restore_umd_profile_pstate_after_runpm(adev);
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 8d1b097..df24b01 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -48,6 +48,7 @@ enum amdgpu_int_thermal_type {
 enum amdgpu_runpm_mode {
 	AMDGPU_RUNPM_NONE,
 	AMDGPU_RUNPM_PX,
+	AMDGPU_RUNPM_GMUX,
 	AMDGPU_RUNPM_BOCO,
 	AMDGPU_RUNPM_BACO,
 	AMDGPU_RUNPM_BAMACO,
-- 
2.55.0

[PATCH v3 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[Andre Eikmeyer]

The HDA controller normally needs different WAKEEN settings for runtime and
system suspend, so azx_prepare() prevents direct complete. For a switcheroo
audio client whose bound GPU is already runtime suspended and switched off,
however, the controller is physically inaccessible and cannot signal wake
events.

Resuming the HDA function during system suspend also acquires its runtime
PM supplier. This powers the discrete GPU back on only to suspend it again
and can expose failures while restoring an otherwise unused GPU.

Direct complete is allowed when the bound VGA device is both runtime
suspended and reported off by vga_switcheroo. Active GPUs and HDA
controllers without a switcheroo binding retain the existing WAKEEN
transition.

This was tested as part of the MacBookPro15,1 hybrid graphics series on
both the 2018 and 2019 revisions. The discrete GPU remains powered off
across system suspend when it is unused, and suspend and resume complete
successfully.

Reviewed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
 sound/hda/controllers/intel.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 28c55c5..194d962 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -334,9 +334,11 @@ enum {
 #ifdef SUPPORT_VGA_SWITCHEROO
 #define use_vga_switcheroo(chip)	((chip)->use_vga_switcheroo)
 #define needs_eld_notify_link(chip)	((chip)->bus.keep_power)
+static bool azx_vga_is_powered_off(struct azx *chip);
 #else
 #define use_vga_switcheroo(chip)	0
 #define needs_eld_notify_link(chip)	false
+#define azx_vga_is_powered_off(chip)	false
 #endif
 
 static const char * const driver_short_names[] = {
@@ -1025,6 +1027,10 @@ static int azx_prepare(struct device *dev)
 		return 0;
 
 	chip = card->private_data;
+	/* A positive return allows the PM core to use direct complete. */
+	if (azx_vga_is_powered_off(chip))
+		return 1;
+
 	chip->pm_prepared = 1;
 	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
 
@@ -1185,6 +1191,24 @@ static int azx_probe_continue(struct azx *chip);
 #ifdef SUPPORT_VGA_SWITCHEROO
 static struct pci_dev *get_bound_vga(struct pci_dev *pci);
 
+static bool azx_vga_is_powered_off(struct azx *chip)
+{
+	struct pci_dev *pci;
+	bool powered_off = false;
+
+	if (!use_vga_switcheroo(container_of(chip, struct hda_intel, chip)))
+		return false;
+
+	pci = get_bound_vga(chip->pci);
+	if (pci) {
+		powered_off = pm_runtime_suspended(&pci->dev) &&
+			vga_switcheroo_get_client_state(pci) == VGA_SWITCHEROO_OFF;
+		pci_dev_put(pci);
+	}
+
+	return powered_off;
+}
+
 static void azx_vs_set_state(struct pci_dev *pci,
 			     enum vga_switcheroo_state state)
 {
-- 
2.55.0

Re: [PATCH v3 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The discrete GPU's PCI device pointer (`discrete_pdev`) fails to initialize if the `apple-gmux` module is reloaded, breaking the discrete GPU power-on sequence on MacBookPro15,1.
--

--- Patch [1]: [PATCH v3 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence ---
commit 82f7efe17d405f437532909cef38c118380b2e3f
Author: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
Subject: platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

This commit adds a specific power-on sequence using PWG1 and PWG3 ACPI
methods for the discrete GPU on MacBookPro15,1. It evaluates these link
methods around the GMUX transition and waits for PCI configuration space
before completing power-on.

> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index fbc30f1f8abdf..2b6363aab8d15 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -510,14 +541,49 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
>  static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data,
>  				   enum vga_switcheroo_state state)
>  {
> +	int ret;
> +
>  	reinit_completion(&gmux_data->powerchange_done);
>  
>  	if (state == VGA_SWITCHEROO_ON) {
> -		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 1);
> -		gmux_write8(gmux_data, GMUX_PORT_DISCRETE_POWER, 3);
> +		if (gmux_data->use_pwg_power_sequence &&
> +		    gmux_data->discrete_pdev) {

[Severity: High]
If discrete_pdev fails to initialize during module reload, won't this check
silently fall back to the legacy power sequence?

If the legacy sequence is used on this model, could the discrete GPU fail to
power on properly, causing subsequent PCI accesses to read 0xFFFFFFFF and
potentially crash the system?

[ ... ]

> @@ -549,11 +615,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
>  	 */
>  	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
>  		return VGA_SWITCHEROO_IGD;
> -	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> -		 pdev->device == 0x0863)
> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
>  		return VGA_SWITCHEROO_IGD;
> -	else
> -		return VGA_SWITCHEROO_DIS;
> +
> +	if (apple_gmux_data->use_pwg_power_sequence &&
> +	    apple_gmux_data->discrete_pdev != pdev) {
> +		pci_dev_put(apple_gmux_data->discrete_pdev);
> +		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
> +	}
> +
> +	return VGA_SWITCHEROO_DIS;
>  }

[Severity: High]
If the apple-gmux module is unloaded and reloaded, will this initialization
actually happen?

Looking at vga_switcheroo_enable() in the vgasr core, it intentionally skips
get_client_id() for clients that already have a known ID:

vga_switcheroo.c:vga_switcheroo_enable() {
    ...
    if (!client_is_vga(client) ||
         client_id(client) != VGA_SWITCHEROO_UNKNOWN_ID)
        continue;

    ret = vgasr_priv.handler->get_client_id(client->pdev);
    ...
}

Since clients retain their IDs from the first load, won't get_client_id() be
skipped entirely on module reload, leaving discrete_pdev as NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144750.36797-1-dev@deq.rocks?part=1

Re: [PATCH v2 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Connecting an external GPU (eGPU) overwrites the internal discrete GPU reference, permanently breaking internal GPU power management.
--

--- Patch [1]: [PATCH v2 1/3] platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence ---
commit 469501c98c440969c62cc4d684e58812e4f65cfa
Author: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>

platform/x86: apple-gmux: add MacBookPro15,1 dGPU power sequence

The discrete GPU on the MacBookPro15,1 does not return after the legacy
GMUX power-on sequence. Evaluate the firmware PWG1 and PWG3 link methods
around the GMUX transition and wait for PCI configuration space before
completing power-on.

> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index fbc30f1f8abdf..ce926a10131e8 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c

[ ... ]

> @@ -549,11 +607,16 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
>  	 */
>  	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
>  		return VGA_SWITCHEROO_IGD;
> -	else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> -		 pdev->device == 0x0863)
> +	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && pdev->device == 0x0863)
>  		return VGA_SWITCHEROO_IGD;
> -	else
> -		return VGA_SWITCHEROO_DIS;
> +
> +	if (apple_gmux_data->use_pwg_power_sequence &&
> +	    apple_gmux_data->discrete_pdev != pdev) {
> +		pci_dev_put(apple_gmux_data->discrete_pdev);
> +		apple_gmux_data->discrete_pdev = pci_dev_get(pdev);
> +	}
> +
> +	return VGA_SWITCHEROO_DIS;
>  }

[Severity: High]
Does connecting an external GPU (eGPU) overwrite the internal discrete GPU
reference here and break internal GPU power management?

When an eGPU is connected via Thunderbolt, its audio controller is probed by
the snd_hda_intel driver. This driver unconditionally calls
vga_switcheroo_register_audio_client(), passing the eGPU as the bound VGA
device.

vga_switcheroo then invokes the gmux_get_client_id() callback with the eGPU's
pdev to resolve the audio client's ID. Because the eGPU is not an integrated
GPU, this new check executes and overwrites apple_gmux_data->discrete_pdev
with the eGPU's pci_dev.

If this happens, subsequent power state changes might attempt to evaluate ACPI
methods like PWG1 and PWG3 on the eGPU instead of the internal dGPU, which
could leave the internal dGPU in an undefined power state and break hybrid
graphics.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144221.35877-1-dev@deq.rocks?part=1

Re: [PATCH v2 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] State leak on runtime PM error paths for GMUX and PX devices.

Pre-existing issues:
- [High] BOCO capability checks incorrectly overlap with PX mode, breaking system suspend.
--

--- Patch [2]: [PATCH v2 2/3] drm/amdgpu: add Apple GMUX runtime PM support ---
commit 0b96cccee1dc38f37893eddd321b413c6829882e
Author: Andre Eikmeyer <dev@deq.rocks>
Subject: drm/amdgpu: add Apple GMUX runtime PM support

This commit adds runtime PM support for the discrete GPU on the Apple
MacBookPro15,1 using the vga_switcheroo PM domain. It allows the dGPU
to enter D3cold while the iGPU is primary and wake on demand.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..a231a002831bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2494,11 +2494,12 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
>  
>  	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
> -		/* only need to skip on ATPX */
>  		if (amdgpu_device_supports_px(adev))
>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> -		/* we want direct complete for BOCO */
> -		if (amdgpu_device_supports_boco(adev))
> +		/* BOCO and GMUX can remain runtime suspended across system sleep. */
> +		if (amdgpu_device_supports_boco(adev) ||
> +		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)

[Severity: High]
This is a pre-existing issue, but does this direct capability check for BOCO
incorrectly evaluate to true for hybrid devices? If a device supports both
PX (ATPX) and BOCO capabilities, it seems amdgpu_device_supports_boco()
will return true in amdgpu_pci_probe(), overwriting the
DPM_FLAG_NO_DIRECT_COMPLETE flag just set for PX.

>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
>  						DPM_FLAG_SMART_SUSPEND |
>  						DPM_FLAG_MAY_SKIP_RESUME);
[ ... ]
> @@ -2609,7 +2610,9 @@ static int amdgpu_pmops_prepare(struct device *dev)
>  	/* Return a positive number here so
>  	 * DPM_FLAG_SMART_SUSPEND works properly
>  	 */
> -	if (amdgpu_device_supports_boco(adev) && pm_runtime_suspended(dev))
> +	if ((amdgpu_device_supports_boco(adev) ||
> +	     adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) &&
> +	    pm_runtime_suspended(dev))

[Severity: High]
This is a pre-existing issue, but does this capability check erroneously
instruct the PM core to skip system suspend and resume for a PX-designated
device? If the device supports both PX and BOCO, returning 1 here in
amdgpu_pmops_prepare() will skip system suspend, which ATPX explicitly
requires.

>  		return 1;
>  
>  	/* if we will not support s3 or s2i for the device
[ ... ]
> @@ -2872,7 +2875,8 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>  	}
>  
>  	adev->in_runpm = true;
> -	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> +	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>  
>  	/*
[ ... ]
>  	ret = amdgpu_device_prepare(drm_dev);
>  	if (ret)
>  		return ret;
>  	ret = amdgpu_device_suspend(drm_dev, false);
>  	if (ret) {
>  		adev->in_runpm = false;
>  		if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
>  			adev->mp1_state = PP_MP1_STATE_NONE;
>  		return ret;
>  	}

[Severity: Medium]
If amdgpu_device_suspend() fails later in amdgpu_pmops_runtime_suspend(),
is drm_dev->switch_power_state left permanently in
DRM_SWITCH_POWER_CHANGING? 

The early return on failure does not appear to restore the previous power
state, and this state leak now applies to the GMUX path as well.

[ ... ]
> @@ -2947,10 +2952,11 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>  	if (!pci_device_is_present(adev->pdev))
>  		adev->no_hw_access = true;
>  
> -	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
> +	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX) {
>  		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>  
> -		/* Only need to handle PCI state in the driver for ATPX
> +		/* Only need to handle PCI state in the driver for ATPX and GMUX.
>  		 * PCI core handles it for _PR3.
>  		 */
>  		pci_set_power_state(pdev, PCI_D0);
> @@ -2970,12 +2976,14 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>  	}
>  	ret = amdgpu_device_resume(drm_dev, false);
>  	if (ret) {
> -		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> +		if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  			pci_disable_device(pdev);
>  		return ret;
>  	}

[Severity: Medium]
If either pci_enable_device() or amdgpu_device_resume() fails and returns
early in amdgpu_pmops_runtime_resume(), does this leave
drm_dev->switch_power_state permanently set to DRM_SWITCH_POWER_CHANGING,
since the failure paths do not restore it?

> -	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> +	if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX ||
> +	    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  		drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
>  
>  	amdgpu_restore_umd_profile_pstate_after_runpm(adev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144221.35877-1-dev@deq.rocks?part=2

Re: [PATCH v3 2/3] drm/amdgpu: add Apple GMUX runtime PM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The system suspend and hibernation poweroff callbacks fail to handle the runtime-suspended state for GMUX devices, leading to unhandled MMIO accesses on powered-off hardware if `direct_complete` is aborted.
--

--- Patch [2]: [PATCH v3 2/3] drm/amdgpu: add Apple GMUX runtime PM support ---
commit bd409ba52fe58e23b69e9e8883bcbcd307329cae
Author: Andre Eikmeyer <dev@deq.rocks>
Subject: drm/amdgpu: add Apple GMUX runtime PM support

The MacBookPro15,1 uses Apple GMUX to control power to the discrete GPU,
but it does not expose the ATPX or ACPI power-resource interfaces currently
recognized by amdgpu. A dedicated GMUX runtime PM mode uses the existing
vga_switcheroo PM domain to sequence GPU power. A runtime-suspended GMUX GPU
can remain powered off across system sleep, following the smart-suspend
handling already used by BOCO devices.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0ab380ca7e645..a231a002831bb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

[ ... ]

> @@ -2494,11 +2494,12 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  		DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
>  
>  	if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
> -		/* only need to skip on ATPX */
> +		/* ATPX requires a full system-sleep transition. */
>  		if (amdgpu_device_supports_px(adev))
>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> -		/* we want direct complete for BOCO */
> -		if (amdgpu_device_supports_boco(adev))
> +		/* BOCO and GMUX can remain runtime suspended across system sleep. */
> +		if (amdgpu_device_supports_boco(adev) ||
> +		    adev->pm.rpm_mode == AMDGPU_RUNPM_GMUX)
>  			dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
>  						DPM_FLAG_SMART_SUSPEND |
>  						DPM_FLAG_MAY_SKIP_RESUME);

[Severity: High]
Does this setup require handling the runtime-suspended state in the system
suspend and hibernation callbacks for GMUX devices?

If a child device like the HDA audio controller aborts direct_complete for
the GPU, or if the GPU supports PME from D3cold, the PCI PM core will skip
runtime-resuming the GPU during system suspend.

This causes amdgpu_pmops_suspend() and amdgpu_pmops_poweroff() in
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c to be called while the GPU is still
in the D3cold state powered off by GMUX.

Since these callbacks unconditionally fall through to amdgpu_device_suspend()
without checking the runtime PM state:

drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pmops_suspend() {
    ...
    return amdgpu_device_suspend(drm_dev, true);
}

and

drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:amdgpu_pmops_poweroff() {
    ...
    if (adev->in_s4 && adev->in_suspend)
        return 0;

    return amdgpu_device_suspend(drm_dev, true);
}

Could this lead to unhandled MMIO accesses on the powered-off hardware
during the suspension phase and cause system hangs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144750.36797-1-dev@deq.rocks?part=2

Re: [PATCH v2 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning 1 early from `azx_prepare()` for direct-complete skips essential system suspend preparation (`chip->pm_prepared = 1`, `snd_power_change_state()`, and `flush_work()`), breaking the fallback path if direct-complete is aborted.
--

--- Patch [3]: [PATCH v2 3/3] ALSA: hda: allow direct complete with a powered-off GPU ---
commit 512712a6e8db78ff16bf65eb2c4538a62401c892
Author: Andre Eikmeyer <dev@deq.rocks>

ALSA: hda: allow direct complete with a powered-off GPU

This commit allows direct complete for the HDA controller when a bound
GPU is powered off and unused. It checks the vga_switcheroo state and
runtime PM status to return 1 from azx_prepare() when appropriate.

> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 4b03c64e72ab4..6f1c1d596e2ca 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c

[ ... ]

> @@ -1025,6 +1027,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A positive return allows the PM core to use direct complete. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +

[Severity: High]
By returning 1 early here in azx_prepare(), does this inadvertently skip
setting chip->pm_prepared = 1, calling snd_power_change_state(), and
flushing the asynchronous work queue?

If the PM core or PCI subsystem subsequently aborts direct-complete (for
instance, if device wakeups are enabled via sysfs or a child codec aborts
it), the fallback path will invoke azx_suspend() and azx_resume() on an
unprepared device.

Could this result in the ALSA card remaining in the D0 state instead of
D3hot, failing to block userspace hardware accesses during system suspend?

Furthermore, since flush_work(&azx_bus(chip)->unsol_work) is skipped, is
there a risk of a race between the asynchronous work queue and hardware
shutdown during azx_suspend()?

Finally, during system resume, if chip->pm_prepared remains 0, won't
__azx_runtime_resume() incorrectly request runtime resumes for codecs while
a system resume is still in progress?

>  	chip->pm_prepared = 1;
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144221.35877-1-dev@deq.rocks?part=3

Re: [PATCH v3 3/3] ALSA: hda: allow direct complete with a powered-off GPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The `azx_prepare` function conditionally returns 1 early to request direct complete, bypassing mandatory ALSA state initialization and workqueue flushing. If direct complete is subsequently aborted by the PM core, the device will suspend in an inconsistent state.

Pre-existing issues:
- [High] The HDA interrupt handler uses a non-atomic runtime PM check, and the runtime suspend callback shuts down the hardware without synchronizing shared IRQs. This allows the interrupt handler to run concurrently while the hardware is being powered down.
--

--- Patch [3]: [PATCH v3 3/3] ALSA: hda: allow direct complete with a powered-off GPU ---
commit 8f27e09b7a8bd094dbb356dbe9893145a18d34a2
Author: Andre Eikmeyer <dev@deq.rocks>

ALSA: hda: allow direct complete with a powered-off GPU

The commit allows direct complete during system suspend for a switcheroo
audio client when the bound GPU is already runtime suspended and switched
off. It does this by checking the VGA switcheroo state and returning 1
from azx_prepare().

> diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
> index 4b03c64e72ab4..6f1c1d596e2ca 100644
> --- a/sound/hda/controllers/intel.c
> +++ b/sound/hda/controllers/intel.c

[ ... ]

> @@ -1025,6 +1027,10 @@ static int azx_prepare(struct device *dev)
>  		return 0;
>  
>  	chip = card->private_data;
> +	/* A positive return allows the PM core to use direct complete. */
> +	if (azx_vga_is_powered_off(chip))
> +		return 1;
> +

[Severity: High]
If the PM core decides to abort direct complete for this device later in the
suspend process, will this leave the device in an inconsistent state?

For example, if a child HDA codec returns 0 from its own prepare callback,
the PM core clears the direct complete flag and will fall back to calling
azx_suspend().

Since this early return bypasses snd_power_change_state() and flush_work(),
the ALSA subsystem will still consider the device to be in SNDRV_CTL_POWER_D0
and the unsol_work may still be queued or running while the hardware gets
shut down.

>  	chip->pm_prepared = 1;
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);

[ ... ]

[Severity: High]
This is a pre-existing issue, but I have a question about the existing
runtime PM handling and shared interrupts in this driver.

Does azx_runtime_suspend() need to call synchronize_irq() before powering
down the hardware?

Currently, azx_interrupt() in sound/hda/common/controller.c uses a
non-atomic pm_runtime_active() check. If a shared interrupt fires for
another device, azx_interrupt() could see the device as active, while
another CPU concurrently executes azx_runtime_suspend() and shuts down the
hardware.

A read from the powered-off hardware would then typically return 0xffffffff.
If this happens between reading INTSTS and RIRBSTS, the 0xff byte read could
incorrectly match RIRB_INT_MASK and trigger further invalid accesses or an
IRQ storm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812144750.36797-1-dev@deq.rocks?part=3