[PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Orlando Chamberlain]

Hi All,

This patch series adds support for the MMIO based gmux present on these
Dual GPU Apple T2 Macs: MacBookPro15,1, MacBookPro15,3, MacBookPro16,1,
MacBookPro16,4 (although amdgpu isn't working on MacBookPro16,4 [1]).

Changes from v2[2]:

- Add "," to last item in apple_gmux_type enum
- Don't not clear interrupts when status is 0
- Don't check if we failed to make debugfs folder
- Check for fake mmio gmux

# 1:

has a slight change in how the switch state is read: instead of checking
for x == 2, check !(x & 1)

# 2:

implements a system to support more than 2 gmux types

# 3:

start using the gmux's GMSP acpi method when handling interrupts on MMIO
gmux's. This is needed for the MMIO gmux's to clear interrupts.

# 4:

Adds support for the MMIO based gmux on T2 macs.

# 5:

Add a debugfs interface to apple-gmux so data from ports can be read
and written to from userspace.

This can be used for more easily researching what unknown ports do,
and switching gpus when vga_switcheroo isn't ready (e.g. when one gpu
is bound to vfio-pci and in use by a Windows VM, I can use this to
switch my internal display between Linux and Windows easily).

# Issues:

1. Switching gpus at runtime has the same issue as indexed gmux's: the
inactive gpu can't probe the DDC lines for eDP [3]

2. iMacPro1,1, iMac20,1 and iMac20,2 all seem to have a gmux in their
acpi tables, but they shouldn't. A check that hopefully will detect this
is used, but it's untested as I don't have any of those computers.

3. Powering on the amdgpu with vga_switcheroo doesn't work well. I'm
told on the MacBookPro15,1 it works sometimes, and adding delays helps,
but on my MacBookPro16,1 I haven't been able to get it to work at all:

amdgpu: switched off
amdgpu: switched on
amdgpu 0000:03:00.0:
    Unable to change power state from D3hot to D0, device inaccessible
amdgpu 0000:03:00.0:
    Unable to change power state from D3cold to D0, device inaccessible
[drm] PCIE GART of 512M enabled (table at 0x00000080FEE00000).
[drm] PSP is resuming...
[drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
[drm:psp_resume [amdgpu]] *ERROR* PSP resume failed
[drm:amdgpu_device_fw_loading [amdgpu]]
    *ERROR* resume of IP block <psp> failed -62
amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_resume failed (-62).
snd_hda_intel 0000:03:00.1: Enabling via vga_switcheroo
snd_hda_intel 0000:03:00.1:
    Unable to change power state from D3cold to D0, device inaccessible
snd_hda_intel 0000:03:00.1: CORB reset timeout#2, CORBRP = 65535
snd_hda_codec_hdmi hdaudioC0D0: Unable to sync register 0x2f0d00. -5

There are some acpi methods (PWRD, PWG1 [4, 5]) that macOS calls when
changing the amdgpu's power state, but we don't use them and that could be
a cause. Additionally unlike previous generation Macbooks which work
better, on MacBookPro16,1 the gpu is located behind 2 pci bridges:

01:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
    Navi 10 XL Upstream Port of PCI Express Switch (rev 43)
02:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
    Navi 10 XL Downstream Port of PCI Express Switch
03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
    Navi 14 [Radeon RX 5500/5500M / Pro 5500M] (rev 43)
03:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI]
    Navi 10 HDMI Audio

Upon attempting to power on the gpu with vga_switcheroo, all these
devices except 01:00.0 have their config space filled with 1s.
Rescanning pci makes the config space of all the devices go back to
normal, however amdgpu still fails to resume with the same logs as
above.

[1]: https://lore.kernel.org/all/3AFB9142-2BD0-46F9-AEA9-C9C5D13E68E6@live.com/
[2]: https://lore.kernel.org/platform-driver-x86/20230216122342.5918-1-orlandoch.dev@gmail.com/
[3]: https://lore.kernel.org/all/9eed8ede6f15a254ad578e783b050e1c585d5a15.1439288957.git.lukas@wunner.de/
[4]: https://gist.github.com/Redecorating/6c7136b7a4ac7ce3b77d8e41740dd87b
[5]: https://lore.kernel.org/all/20120710160555.GA31562@srcf.ucam.org/

Orlando Chamberlain (5):
  apple-gmux: use first bit to check switch state
  apple-gmux: refactor gmux types
  apple-gmux: Use GMSP acpi method for interrupt clear
  apple-gmux: support MMIO gmux on T2 Macs
  apple-gmux: add debugfs interface

 drivers/platform/x86/apple-gmux.c | 349 ++++++++++++++++++++++++++----
 include/linux/apple-gmux.h        |  70 ++++--
 2 files changed, 357 insertions(+), 62 deletions(-)

-- 
2.39.1

[PATCH v3 1/5] apple-gmux: use first bit to check switch state

[Orlando Chamberlain]

On T2 Macs with MMIO gmux, when GMUX_PORT_SWITCH_DISPLAY is read, it can
have values of 2, 3, 4, and 5. Odd values correspond to the discrete gpu,
and even values correspond to the integrated gpu. The current logic is
that only 2 corresponds to IGD, but this doesn't work for T2 Macs.
Instead, check the first bit to determine the connected gpu.

As T2 Macs with gmux only can switch the internal display, it is
untested if this change (or a similar change) would be applicable
to GMUX_PORT_SWITCH_DDC and GMUX_PORT_SWITCH_EXTERNAL.

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
---
 drivers/platform/x86/apple-gmux.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9333f82cfa8a..ec99e05e532c 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -346,10 +346,10 @@ static void gmux_read_switch_state(struct apple_gmux_data *gmux_data)
 	else
 		gmux_data->switch_state_ddc = VGA_SWITCHEROO_DIS;
 
-	if (gmux_read8(gmux_data, GMUX_PORT_SWITCH_DISPLAY) == 2)
-		gmux_data->switch_state_display = VGA_SWITCHEROO_IGD;
-	else
+	if (gmux_read8(gmux_data, GMUX_PORT_SWITCH_DISPLAY) & 1)
 		gmux_data->switch_state_display = VGA_SWITCHEROO_DIS;
+	else
+		gmux_data->switch_state_display = VGA_SWITCHEROO_IGD;
 
 	if (gmux_read8(gmux_data, GMUX_PORT_SWITCH_EXTERNAL) == 2)
 		gmux_data->switch_state_external = VGA_SWITCHEROO_IGD;
-- 
2.39.1

[PATCH v3 2/5] apple-gmux: refactor gmux types

[Orlando Chamberlain]

Add apple_gmux_config struct containing operations and data specific to
each mux type.

This is in preparation for adding a third, MMIO based, gmux type.

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
---
v2->v3: add comma to last item in enum apple_gmux_type
 drivers/platform/x86/apple-gmux.c | 93 ++++++++++++++++++++-----------
 include/linux/apple-gmux.h        | 18 ++++--
 2 files changed, 74 insertions(+), 37 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index ec99e05e532c..36208e93d745 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -5,6 +5,7 @@
  *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
  *  Copyright (C) 2010-2012 Andreas Heider <andreas@meetr.de>
  *  Copyright (C) 2015 Lukas Wunner <lukas@wunner.de>
+ *  Copyright (C) 2023 Orlando Chamberlain <orlandoch.dev@gmail.com>
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -43,10 +44,12 @@
  *     http://www.renesas.com/products/mpumcu/h8s/h8s2100/h8s2113/index.jsp
  */
 
+struct apple_gmux_config;
+
 struct apple_gmux_data {
 	unsigned long iostart;
 	unsigned long iolen;
-	bool indexed;
+	const struct apple_gmux_config *config;
 	struct mutex index_lock;
 
 	struct backlight_device *bdev;
@@ -64,6 +67,18 @@ struct apple_gmux_data {
 
 static struct apple_gmux_data *apple_gmux_data;
 
+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);
+	u32 (*read32)(struct apple_gmux_data *gmux_data, int port);
+	void (*write32)(struct apple_gmux_data *gmux_data, int port, u32 val);
+	const struct vga_switcheroo_handler *gmux_handler;
+	enum vga_switcheroo_handler_flags_t handler_flags;
+	unsigned long resource_type;
+	bool read_version_as_u32;
+	char *name;
+};
+
 #define GMUX_INTERRUPT_ENABLE		0xff
 #define GMUX_INTERRUPT_DISABLE		0x00
 
@@ -195,35 +210,23 @@ static void gmux_index_write32(struct apple_gmux_data *gmux_data, int port,
 
 static u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
 {
-	if (gmux_data->indexed)
-		return gmux_index_read8(gmux_data, port);
-	else
-		return gmux_pio_read8(gmux_data, port);
+	return gmux_data->config->read8(gmux_data, port);
 }
 
 static void gmux_write8(struct apple_gmux_data *gmux_data, int port, u8 val)
 {
-	if (gmux_data->indexed)
-		gmux_index_write8(gmux_data, port, val);
-	else
-		gmux_pio_write8(gmux_data, port, val);
+	return gmux_data->config->write8(gmux_data, port, val);
 }
 
 static u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
 {
-	if (gmux_data->indexed)
-		return gmux_index_read32(gmux_data, port);
-	else
-		return gmux_pio_read32(gmux_data, port);
+	return gmux_data->config->read32(gmux_data, port);
 }
 
 static void gmux_write32(struct apple_gmux_data *gmux_data, int port,
 			     u32 val)
 {
-	if (gmux_data->indexed)
-		gmux_index_write32(gmux_data, port, val);
-	else
-		gmux_pio_write32(gmux_data, port, val);
+	return gmux_data->config->write32(gmux_data, port, val);
 }
 
 /**
@@ -463,19 +466,43 @@ static enum vga_switcheroo_client_id gmux_get_client_id(struct pci_dev *pdev)
 		return VGA_SWITCHEROO_DIS;
 }
 
-static const struct vga_switcheroo_handler gmux_handler_indexed = {
+static const struct vga_switcheroo_handler gmux_handler_no_ddc = {
 	.switchto = gmux_switchto,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,
 };
 
-static const struct vga_switcheroo_handler gmux_handler_classic = {
+static const struct vga_switcheroo_handler gmux_handler_ddc = {
 	.switchto = gmux_switchto,
 	.switch_ddc = gmux_switch_ddc,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,
 };
 
+static const struct apple_gmux_config apple_gmux_pio = {
+	.read8 = &gmux_pio_read8,
+	.write8 = &gmux_pio_write8,
+	.read32 = &gmux_pio_read32,
+	.write32 = &gmux_pio_write32,
+	.gmux_handler = &gmux_handler_ddc,
+	.handler_flags = VGA_SWITCHEROO_CAN_SWITCH_DDC,
+	.resource_type = IORESOURCE_IO,
+	.read_version_as_u32 = false,
+	.name = "classic"
+};
+
+static const struct apple_gmux_config apple_gmux_index = {
+	.read8 = &gmux_index_read8,
+	.write8 = &gmux_index_write8,
+	.read32 = &gmux_index_read32,
+	.write32 = &gmux_index_write32,
+	.gmux_handler = &gmux_handler_no_ddc,
+	.handler_flags = VGA_SWITCHEROO_NEEDS_EDP_CONFIG,
+	.resource_type = IORESOURCE_IO,
+	.read_version_as_u32 = true,
+	.name = "indexed"
+};
+
 /**
  * DOC: Interrupt
  *
@@ -565,13 +592,13 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	int ret = -ENXIO;
 	acpi_status status;
 	unsigned long long gpe;
-	bool indexed = false;
+	enum apple_gmux_type type;
 	u32 version;
 
 	if (apple_gmux_data)
 		return -EBUSY;
 
-	if (!apple_gmux_detect(pnp, &indexed)) {
+	if (!apple_gmux_detect(pnp, &type)) {
 		pr_info("gmux device not present\n");
 		return -ENODEV;
 	}
@@ -581,6 +608,16 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		return -ENOMEM;
 	pnp_set_drvdata(pnp, gmux_data);
 
+	switch (type) {
+	case APPLE_GMUX_TYPE_INDEXED:
+		gmux_data->config = &apple_gmux_index;
+		mutex_init(&gmux_data->index_lock);
+		break;
+	case APPLE_GMUX_TYPE_PIO:
+		gmux_data->config = &apple_gmux_pio;
+		break;
+	}
+
 	res = pnp_get_resource(pnp, IORESOURCE_IO, 0);
 	gmux_data->iostart = res->start;
 	gmux_data->iolen = resource_size(res);
@@ -591,9 +628,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		goto err_free;
 	}
 
-	if (indexed) {
-		mutex_init(&gmux_data->index_lock);
-		gmux_data->indexed = true;
+	if (gmux_data->config->read_version_as_u32) {
 		version = gmux_read32(gmux_data, GMUX_PORT_VERSION_MAJOR);
 		ver_major = (version >> 24) & 0xff;
 		ver_minor = (version >> 16) & 0xff;
@@ -604,7 +639,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);
 	}
 	pr_info("Found gmux version %d.%d.%d [%s]\n", ver_major, ver_minor,
-		ver_release, (gmux_data->indexed ? "indexed" : "classic"));
+		ver_release, gmux_data->config->name);
 
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_PLATFORM;
@@ -694,12 +729,8 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	 *
 	 * Pre-retina MacBook Pros can switch the panel's DDC separately.
 	 */
-	if (gmux_data->indexed)
-		ret = vga_switcheroo_register_handler(&gmux_handler_indexed,
-					      VGA_SWITCHEROO_NEEDS_EDP_CONFIG);
-	else
-		ret = vga_switcheroo_register_handler(&gmux_handler_classic,
-					      VGA_SWITCHEROO_CAN_SWITCH_DDC);
+	ret = vga_switcheroo_register_handler(gmux_data->config->gmux_handler,
+			gmux_data->config->handler_flags);
 	if (ret) {
 		pr_err("Failed to register vga_switcheroo handler\n");
 		goto err_register_handler;
diff --git a/include/linux/apple-gmux.h b/include/linux/apple-gmux.h
index 1f68b49bcd68..147dc1c52e08 100644
--- a/include/linux/apple-gmux.h
+++ b/include/linux/apple-gmux.h
@@ -36,6 +36,11 @@
 
 #define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)
 
+enum apple_gmux_type {
+	APPLE_GMUX_TYPE_PIO,
+	APPLE_GMUX_TYPE_INDEXED,
+};
+
 #if IS_ENABLED(CONFIG_APPLE_GMUX)
 static inline bool apple_gmux_is_indexed(unsigned long iostart)
 {
@@ -65,13 +70,13 @@ static inline bool apple_gmux_is_indexed(unsigned long iostart)
  * Return: %true if a supported gmux ACPI device is detected and the kernel
  * was configured with CONFIG_APPLE_GMUX, %false otherwise.
  */
-static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, bool *indexed_ret)
+static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, enum apple_gmux_type *type_ret)
 {
 	u8 ver_major, ver_minor, ver_release;
 	struct device *dev = NULL;
 	struct acpi_device *adev;
 	struct resource *res;
-	bool indexed = false;
+	enum apple_gmux_type type = APPLE_GMUX_TYPE_PIO;
 	bool ret = false;
 
 	if (!pnp_dev) {
@@ -99,13 +104,14 @@ static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, bool *indexed_ret)
 	ver_minor = inb(res->start + GMUX_PORT_VERSION_MINOR);
 	ver_release = inb(res->start + GMUX_PORT_VERSION_RELEASE);
 	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
-		indexed = apple_gmux_is_indexed(res->start);
-		if (!indexed)
+		if (apple_gmux_is_indexed(res->start))
+			type = APPLE_GMUX_TYPE_INDEXED;
+		else
 			goto out;
 	}
 
-	if (indexed_ret)
-		*indexed_ret = indexed;
+	if (type_ret)
+		*type_ret = type;
 
 	ret = true;
 out:
-- 
2.39.1

[PATCH v3 3/5] apple-gmux: Use GMSP acpi method for interrupt clear

[Orlando Chamberlain]

This is needed for interrupts to be cleared correctly on MMIO based
gmux's. It is untested if this helps/hinders other gmux types, so
currently this is only enabled for the MMIO gmux's.

There is also a "GMLV" acpi method, and the "GMSP" method can be called
with 1 as its argument, but the purposes of these aren't known and they
don't seem to be needed.

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>

---
v2->v3: remove status != 0 check
 drivers/platform/x86/apple-gmux.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 36208e93d745..8dfa1c02be64 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -76,6 +76,7 @@ struct apple_gmux_config {
 	enum vga_switcheroo_handler_flags_t handler_flags;
 	unsigned long resource_type;
 	bool read_version_as_u32;
+	bool use_acpi_gmsp;
 	char *name;
 };
 
@@ -488,6 +489,7 @@ static const struct apple_gmux_config apple_gmux_pio = {
 	.handler_flags = VGA_SWITCHEROO_CAN_SWITCH_DDC,
 	.resource_type = IORESOURCE_IO,
 	.read_version_as_u32 = false,
+	.use_acpi_gmsp = false,
 	.name = "classic"
 };
 
@@ -500,6 +502,7 @@ static const struct apple_gmux_config apple_gmux_index = {
 	.handler_flags = VGA_SWITCHEROO_NEEDS_EDP_CONFIG,
 	.resource_type = IORESOURCE_IO,
 	.read_version_as_u32 = true,
+	.use_acpi_gmsp = false,
 	.name = "indexed"
 };
 
@@ -511,8 +514,29 @@ static const struct apple_gmux_config apple_gmux_index = {
  * MCP79, on all following generations it's GPIO pin 6 of the Intel PCH.
  * The GPE merely signals that an interrupt occurred, the actual type of event
  * is identified by reading a gmux register.
+ *
+ * On MMIO gmux's, we also need to call the acpi method GMSP to properly clear
+ * interrupts.
  */
 
+static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data, int arg)
+{
+	acpi_status status = AE_OK;
+	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
+	struct acpi_object_list arg_list = { 1, &arg0 };
+
+	arg0.integer.value = arg;
+
+	status = acpi_evaluate_object(gmux_data->dhandle, "GMSP", &arg_list, NULL);
+	if (ACPI_FAILURE(status)) {
+		pr_err("GMSP call failed: %s\n",
+		       acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static inline void gmux_disable_interrupts(struct apple_gmux_data *gmux_data)
 {
 	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_ENABLE,
@@ -537,6 +561,8 @@ static void gmux_clear_interrupts(struct apple_gmux_data *gmux_data)
 	/* to clear interrupts write back current status */
 	status = gmux_interrupt_get_status(gmux_data);
 	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
+	if (gmux_data->config->use_acpi_gmsp)
+		gmux_call_acpi_gmsp(gmux_data, 0);
 }
 
 static void gmux_notify_handler(acpi_handle device, u32 value, void *context)
-- 
2.39.1

Re: [PATCH v3 3/5] apple-gmux: Use GMSP acpi method for interrupt clear

[Lukas Wunner]

On Sun, Feb 19, 2023 at 12:20:05AM +1100, Orlando Chamberlain wrote:
> This is needed for interrupts to be cleared correctly on MMIO based
> gmux's. It is untested if this helps/hinders other gmux types, so
> currently this is only enabled for the MMIO gmux's.
> 
> There is also a "GMLV" acpi method, and the "GMSP" method can be called
> with 1 as its argument, but the purposes of these aren't known and they
> don't seem to be needed.

GMLV and GMSP access a GPIO on the PCH which is connected to the
GMUX_INT pin of the gmux microcontroller.  I've just verified that
in the schematics of my MBP9,1.

GMLV reads the value of the GPIO ("level").
GMSP likely sets the value ("set polarity").

On my MBP9,1 (indexed gmux), if the gmux controller signals an interrupt,
the platform signals a notification:

  Scope (\_GPE)
  {
      Method (_L16, 0, NotSerialized)  // _Lxx: Level-Triggered GPE
      {
          Notify (\_SB.PCI0.LPCB.GMUX, 0x80) // Status Change
      }
  }

Comparing this to the MBP13,3 and MBP16,1, the GPE method differentiates
between the OS type:  On Darwin, only a notification is signaled,
whereas on other OSes, the GPIO's value is read and then inverted:

  Scope (\_GPE)
  {
      Method (_L15, 0, NotSerialized)  // _Lxx: Level-Triggered GPE, xx=0x00-0xFF
      {
          If (OSDW ())
          {
              Notify (\_SB.PCI0.LPCB.GPUC, 0x80) // Status Change
          }
          ElseIf ((\_SB.GGII (0x03000015) == One))
          {
              \_SB.SGII (0x03000015, Zero)
          }
          Else
          {
              \_SB.SGII (0x03000015, One)
          }
      }
  }

Linux masquerades as Darwin, so ends up in the notification-only
code path.

Does macOS execute the GMSP method as well?  Have you disassembled
the gmux driver?  All vital information that belongs in the commit
message and/or a code comment.


> +static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data, int arg)
> +{
> +	acpi_status status = AE_OK;
> +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> +	struct acpi_object_list arg_list = { 1, &arg0 };
> +
> +	arg0.integer.value = arg;
> +
> +	status = acpi_evaluate_object(gmux_data->dhandle, "GMSP", &arg_list, NULL);

Can this be simplified by using acpi_execute_simple_method() or
one of the other helpers provided by drivers/acpi/utils.c?


> @@ -537,6 +561,8 @@ static void gmux_clear_interrupts(struct apple_gmux_data *gmux_data)
>  	/* to clear interrupts write back current status */
>  	status = gmux_interrupt_get_status(gmux_data);
>  	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
> +	if (gmux_data->config->use_acpi_gmsp)
> +		gmux_call_acpi_gmsp(gmux_data, 0);
>  }

I think it would be clearer to check the gmux type directly here,
so that a casual reader understands that invoking the method is
necessary on MMIO-accessed GMUXes, but not any of the other types.
By contrast, with the use_acpi_gmsp one has to look up first which
of the gmux types sets this to true.

What happens if GMSP is not executed?  Needs to be documented in the
commit message and/or a code comment!

Thanks,

Lukas

Re: [PATCH v3 3/5] apple-gmux: Use GMSP acpi method for interrupt clear

[Orlando Chamberlain]

On Sun, 19 Feb 2023 23:17:37 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> On Sun, Feb 19, 2023 at 12:20:05AM +1100, Orlando Chamberlain wrote:
> > This is needed for interrupts to be cleared correctly on MMIO based
> > gmux's. It is untested if this helps/hinders other gmux types, so
> > currently this is only enabled for the MMIO gmux's.
> > 
> > There is also a "GMLV" acpi method, and the "GMSP" method can be
> > called with 1 as its argument, but the purposes of these aren't
> > known and they don't seem to be needed.  
> 
> GMLV and GMSP access a GPIO on the PCH which is connected to the
> GMUX_INT pin of the gmux microcontroller.  I've just verified that
> in the schematics of my MBP9,1.
> 
> GMLV reads the value of the GPIO ("level").
> GMSP likely sets the value ("set polarity").
> 
> On my MBP9,1 (indexed gmux), if the gmux controller signals an
> interrupt, the platform signals a notification:
> 
>   Scope (\_GPE)
>   {
>       Method (_L16, 0, NotSerialized)  // _Lxx: Level-Triggered GPE
>       {
>           Notify (\_SB.PCI0.LPCB.GMUX, 0x80) // Status Change
>       }
>   }
> 
> Comparing this to the MBP13,3 and MBP16,1, the GPE method
> differentiates between the OS type:  On Darwin, only a notification
> is signaled, whereas on other OSes, the GPIO's value is read and then
> inverted:
> 
>   Scope (\_GPE)
>   {
>       Method (_L15, 0, NotSerialized)  // _Lxx: Level-Triggered GPE,
> xx=0x00-0xFF {
>           If (OSDW ())
>           {
>               Notify (\_SB.PCI0.LPCB.GPUC, 0x80) // Status Change
>           }
>           ElseIf ((\_SB.GGII (0x03000015) == One))
>           {
>               \_SB.SGII (0x03000015, Zero)
>           }
>           Else
>           {
>               \_SB.SGII (0x03000015, One)
>           }
>       }
>   }
> 
> Linux masquerades as Darwin, so ends up in the notification-only
> code path.
> 
> Does macOS execute the GMSP method as well?  Have you disassembled
> the gmux driver?  All vital information that belongs in the commit
> message and/or a code comment.

I think it does, if certain based bits in "HWFeatureMask" (which shows
up in `ioreg -l`) are set, but I'm not very good at RE so I don't know
exactly how macOS uses it. The kext is AppleMuxControl2.kext.

> 
> 
> > +static int gmux_call_acpi_gmsp(struct apple_gmux_data *gmux_data,
> > int arg) +{
> > +	acpi_status status = AE_OK;
> > +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > +	struct acpi_object_list arg_list = { 1, &arg0 };
> > +
> > +	arg0.integer.value = arg;
> > +
> > +	status = acpi_evaluate_object(gmux_data->dhandle, "GMSP",
> > &arg_list, NULL);  
> 
> Can this be simplified by using acpi_execute_simple_method() or
> one of the other helpers provided by drivers/acpi/utils.c?
> 

Yes it can thanks!

> 
> > @@ -537,6 +561,8 @@ static void gmux_clear_interrupts(struct
> > apple_gmux_data *gmux_data) /* to clear interrupts write back
> > current status */ status = gmux_interrupt_get_status(gmux_data);
> >  	gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_STATUS, status);
> > +	if (gmux_data->config->use_acpi_gmsp)
> > +		gmux_call_acpi_gmsp(gmux_data, 0);
> >  }  
> 
> I think it would be clearer to check the gmux type directly here,
> so that a casual reader understands that invoking the method is
> necessary on MMIO-accessed GMUXes, but not any of the other types.
> By contrast, with the use_acpi_gmsp one has to look up first which
> of the gmux types sets this to true.

I can do it like that next version.

> 
> What happens if GMSP is not executed?  Needs to be documented in the
> commit message and/or a code comment!
> 

It gets a flood of status=0 interrupts, I'll add that as a comment.

> Thanks,
> 
> Lukas

[PATCH v3 4/5] apple-gmux: support MMIO gmux on T2 Macs

[Orlando Chamberlain]

In some newer dual gpu MacBooks, gmux is controlled by the T2 security
chip, and acessed with MMIO. Add support for these gmux controllers

Interestingly, the ACPI table only allocates 8 bytes for GMUX, but we
actually need 16, and as such we request 16 with request_mem_region.

Reading and writing from ports:
    16 bytes from 0xfe0b0200 are used. 0x0 to 0x4 are where data
    to read appears, and where data to write goes. Writing to 0xe
    sets the gmux port being accessed, and writing to 0xf sends commands.

    These commands are 0x40 & data_length for write, and data_length for
    read, where data_length is 1, 2 or 4. Once byte base+0xf is 0, the
    command is done.

Issues:
    As with other retina models, we can't switch DDC lines so
    switching at runtime doesn't work if the inactive gpu driver
    already disabled eDP due to it not being connected when that
    driver loaded.

    Additionally, turning on the dgpu back on the MacBookPro16,1 does
    not work.

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
---
v2->v3: add comma after last item in apple_gmux_type enum
v2->v3: check for not present mmio gmux
 drivers/platform/x86/apple-gmux.c | 142 +++++++++++++++++++++++++++---
 include/linux/apple-gmux.h        |  56 +++++++++---
 2 files changed, 174 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 8dfa1c02be64..01e7b1939916 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -28,15 +28,17 @@
  * DOC: Overview
  *
  * gmux is a microcontroller built into the MacBook Pro to support dual GPUs:
- * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on retinas.
+ * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on pre-T2 retinas.
+ * The chip used on T2 Macs is not known.
  *
  * (The MacPro6,1 2013 also has a gmux, however it is unclear why since it has
  * dual GPUs but no built-in display.)
  *
  * gmux is connected to the LPC bus of the southbridge. Its I/O ports are
  * accessed differently depending on the microcontroller: Driver functions
- * to access a pre-retina gmux are infixed ``_pio_``, those for a retina gmux
- * are infixed ``_index_``.
+ * to access a pre-retina gmux are infixed ``_pio_``, those for a pre-T2
+ * retina gmux are infixed ``_index_``, and those on T2 Macs are infixed
+ * with ``_mmio_``.
  *
  * .. _Lattice XP2:
  *     http://www.latticesemi.com/en/Products/FPGAandCPLD/LatticeXP2.aspx
@@ -47,6 +49,7 @@
 struct apple_gmux_config;
 
 struct apple_gmux_data {
+	u8 *__iomem iomem_base;
 	unsigned long iostart;
 	unsigned long iolen;
 	const struct apple_gmux_config *config;
@@ -209,6 +212,79 @@ static void gmux_index_write32(struct apple_gmux_data *gmux_data, int port,
 	mutex_unlock(&gmux_data->index_lock);
 }
 
+static int gmux_mmio_wait(struct apple_gmux_data *gmux_data)
+{
+	int i = 200;
+	u8 gwr = ioread8(gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+
+	while (i && gwr) {
+		gwr = ioread8(gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+		udelay(100);
+		i--;
+	}
+
+	return !!i;
+}
+
+static u8 gmux_mmio_read8(struct apple_gmux_data *gmux_data, int port)
+{
+	u8 val;
+
+	mutex_lock(&gmux_data->index_lock);
+	gmux_mmio_wait(gmux_data);
+	iowrite8((port & 0xff), gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
+	iowrite8(GMUX_MMIO_READ | sizeof(val),
+		gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+	gmux_mmio_wait(gmux_data);
+	val = ioread8(gmux_data->iomem_base);
+	mutex_unlock(&gmux_data->index_lock);
+
+	return val;
+}
+
+static void gmux_mmio_write8(struct apple_gmux_data *gmux_data, int port,
+			      u8 val)
+{
+	mutex_lock(&gmux_data->index_lock);
+	gmux_mmio_wait(gmux_data);
+	iowrite8(val, gmux_data->iomem_base);
+
+	iowrite8(port & 0xff, gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
+	iowrite8(GMUX_MMIO_WRITE | sizeof(val),
+		gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+
+	gmux_mmio_wait(gmux_data);
+	mutex_unlock(&gmux_data->index_lock);
+}
+
+static u32 gmux_mmio_read32(struct apple_gmux_data *gmux_data, int port)
+{
+	u32 val;
+
+	mutex_lock(&gmux_data->index_lock);
+	gmux_mmio_wait(gmux_data);
+	iowrite8((port & 0xff), gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
+	iowrite8(GMUX_MMIO_READ | sizeof(val),
+		gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+	gmux_mmio_wait(gmux_data);
+	val = be32_to_cpu(ioread32(gmux_data->iomem_base));
+	mutex_unlock(&gmux_data->index_lock);
+
+	return val;
+}
+
+static void gmux_mmio_write32(struct apple_gmux_data *gmux_data, int port,
+			       u32 val)
+{
+	mutex_lock(&gmux_data->index_lock);
+	iowrite32(cpu_to_be32(val), gmux_data->iomem_base);
+	iowrite8(port & 0xff, gmux_data->iomem_base + GMUX_MMIO_PORT_SELECT);
+	iowrite8(GMUX_MMIO_WRITE | sizeof(val),
+		gmux_data->iomem_base + GMUX_MMIO_COMMAND_SEND);
+	gmux_mmio_wait(gmux_data);
+	mutex_unlock(&gmux_data->index_lock);
+}
+
 static u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
 {
 	return gmux_data->config->read8(gmux_data, port);
@@ -237,8 +313,8 @@ static void gmux_write32(struct apple_gmux_data *gmux_data, int port,
  * the GPU. On dual GPU MacBook Pros by contrast, either GPU may be suspended
  * to conserve energy. Hence the PWM signal needs to be generated by a separate
  * backlight driver which is controlled by gmux. The earliest generation
- * MBP5 2008/09 uses a `TI LP8543`_ backlight driver. All newer models
- * use a `TI LP8545`_.
+ * MBP5 2008/09 uses a `TI LP8543`_ backlight driver. Newer models
+ * use a `TI LP8545`_ or a TI LP8548.
  *
  * .. _TI LP8543: https://www.ti.com/lit/ds/symlink/lp8543.pdf
  * .. _TI LP8545: https://www.ti.com/lit/ds/symlink/lp8545.pdf
@@ -302,8 +378,8 @@ static const struct backlight_ops gmux_bl_ops = {
  * connecting it either to the discrete GPU or the Thunderbolt controller.
  * Oddly enough, while the full port is no longer switchable, AUX and HPD
  * are still switchable by way of an `NXP CBTL03062`_ (on pre-retinas
- * MBP8 2011 and MBP9 2012) or two `TI TS3DS10224`_ (on retinas) under the
- * control of gmux. Since the integrated GPU is missing the main link,
+ * MBP8 2011 and MBP9 2012) or two `TI TS3DS10224`_ (on pre-t2 retinas) under
+ * the control of gmux. Since the integrated GPU is missing the main link,
  * external displays appear to it as phantoms which fail to link-train.
  *
  * gmux receives the HPD signal of all display connectors and sends an
@@ -506,6 +582,20 @@ static const struct apple_gmux_config apple_gmux_index = {
 	.name = "indexed"
 };
 
+static const struct apple_gmux_config apple_gmux_mmio = {
+	.read8 = &gmux_mmio_read8,
+	.write8 = &gmux_mmio_write8,
+	.read32 = &gmux_mmio_read32,
+	.write32 = &gmux_mmio_write32,
+	.gmux_handler = &gmux_handler_no_ddc,
+	.handler_flags = VGA_SWITCHEROO_NEEDS_EDP_CONFIG,
+	.resource_type = IORESOURCE_MEM,
+	.read_version_as_u32 = true,
+	.use_acpi_gmsp = true,
+	.name = "T2"
+};
+
+
 /**
  * DOC: Interrupt
  *
@@ -635,6 +725,25 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	pnp_set_drvdata(pnp, gmux_data);
 
 	switch (type) {
+	case APPLE_GMUX_TYPE_MMIO:
+		gmux_data->config = &apple_gmux_mmio;
+		mutex_init(&gmux_data->index_lock);
+
+		res = pnp_get_resource(pnp, IORESOURCE_MEM, 0);
+		gmux_data->iostart = res->start;
+		/* Although the ACPI table only allocates 8 bytes, we need 16. */
+		gmux_data->iolen = 16;
+		if (!request_mem_region(gmux_data->iostart, gmux_data->iolen,
+					"Apple gmux")) {
+			pr_err("gmux I/O already in use\n");
+			goto err_free;
+		}
+		gmux_data->iomem_base = ioremap(gmux_data->iostart, gmux_data->iolen);
+		if (!gmux_data->iomem_base) {
+			pr_err("couldn't remap gmux mmio region");
+			goto err_release;
+		}
+		goto get_version;
 	case APPLE_GMUX_TYPE_INDEXED:
 		gmux_data->config = &apple_gmux_index;
 		mutex_init(&gmux_data->index_lock);
@@ -654,6 +763,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		goto err_free;
 	}
 
+get_version:
 	if (gmux_data->config->read_version_as_u32) {
 		version = gmux_read32(gmux_data, GMUX_PORT_VERSION_MAJOR);
 		ver_major = (version >> 24) & 0xff;
@@ -684,7 +794,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 					 gmux_data, &gmux_bl_ops, &props);
 	if (IS_ERR(bdev)) {
 		ret = PTR_ERR(bdev);
-		goto err_release;
+		goto err_unmap;
 	}
 
 	gmux_data->bdev = bdev;
@@ -751,7 +861,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	/*
 	 * Retina MacBook Pros cannot switch the panel's AUX separately
 	 * and need eDP pre-calibration. They are distinguishable from
-	 * pre-retinas by having an "indexed" gmux.
+	 * pre-retinas by having an "indexed" or "T2" gmux.
 	 *
 	 * Pre-retina MacBook Pros can switch the panel's DDC separately.
 	 */
@@ -776,8 +886,14 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 					   &gmux_notify_handler);
 err_notify:
 	backlight_device_unregister(bdev);
+err_unmap:
+	if (gmux_data->iomem_base)
+		iounmap(gmux_data->iomem_base);
 err_release:
-	release_region(gmux_data->iostart, gmux_data->iolen);
+	if (gmux_data->config->resource_type == IORESOURCE_MEM)
+		release_mem_region(gmux_data->iostart, gmux_data->iolen);
+	else
+		release_region(gmux_data->iostart, gmux_data->iolen);
 err_free:
 	kfree(gmux_data);
 	return ret;
@@ -798,7 +914,11 @@ static void gmux_remove(struct pnp_dev *pnp)
 
 	backlight_device_unregister(gmux_data->bdev);
 
-	release_region(gmux_data->iostart, gmux_data->iolen);
+	if (gmux_data->iomem_base) {
+		iounmap(gmux_data->iomem_base);
+		release_mem_region(gmux_data->iostart, gmux_data->iolen);
+	} else
+		release_region(gmux_data->iostart, gmux_data->iolen);
 	apple_gmux_data = NULL;
 	kfree(gmux_data);
 
diff --git a/include/linux/apple-gmux.h b/include/linux/apple-gmux.h
index 147dc1c52e08..0bcb331550a4 100644
--- a/include/linux/apple-gmux.h
+++ b/include/linux/apple-gmux.h
@@ -34,11 +34,18 @@
 #define GMUX_PORT_READ			0xd0
 #define GMUX_PORT_WRITE			0xd4
 
+#define GMUX_MMIO_PORT_SELECT		0x0e
+#define GMUX_MMIO_COMMAND_SEND		0x0f
+
+#define GMUX_MMIO_READ			0x00
+#define GMUX_MMIO_WRITE			0x40
+
 #define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)
 
 enum apple_gmux_type {
 	APPLE_GMUX_TYPE_PIO,
 	APPLE_GMUX_TYPE_INDEXED,
+	APPLE_GMUX_TYPE_MMIO,
 };
 
 #if IS_ENABLED(CONFIG_APPLE_GMUX)
@@ -57,6 +64,24 @@ static inline bool apple_gmux_is_indexed(unsigned long iostart)
 	return false;
 }
 
+static inline bool apple_gmux_is_mmio(unsigned long iostart)
+{
+	u8 *__iomem iomem_base = ioremap(iostart, 16);
+	u8 val;
+
+	if (!iomem_base)
+		return false;
+
+	/*
+	 * If this is 0xff, then gmux must not be present, as the gmux would
+	 * reset it to 0x00, or it would be one of 0x1, 0x4, 0x41, 0x44 if a
+	 * command is currently being processed.
+	 */
+	val = ioread8(iomem_base + GMUX_MMIO_COMMAND_SEND);
+	iounmap(iomem_base);
+	return (val != 0xff);
+}
+
 /**
  * apple_gmux_detect() - detect if gmux is built into the machine
  *
@@ -93,19 +118,24 @@ static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, enum apple_gmux_ty
 	}
 
 	res = pnp_get_resource(pnp_dev, IORESOURCE_IO, 0);
-	if (!res || resource_size(res) < GMUX_MIN_IO_LEN)
-		goto out;
-
-	/*
-	 * Invalid version information may indicate either that the gmux
-	 * device isn't present or that it's a new one that uses indexed io.
-	 */
-	ver_major = inb(res->start + GMUX_PORT_VERSION_MAJOR);
-	ver_minor = inb(res->start + GMUX_PORT_VERSION_MINOR);
-	ver_release = inb(res->start + GMUX_PORT_VERSION_RELEASE);
-	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
-		if (apple_gmux_is_indexed(res->start))
-			type = APPLE_GMUX_TYPE_INDEXED;
+	if (res && resource_size(res) >= GMUX_MIN_IO_LEN) {
+		/*
+		 * Invalid version information may indicate either that the gmux
+		 * device isn't present or that it's a new one that uses indexed io.
+		 */
+		ver_major = inb(res->start + GMUX_PORT_VERSION_MAJOR);
+		ver_minor = inb(res->start + GMUX_PORT_VERSION_MINOR);
+		ver_release = inb(res->start + GMUX_PORT_VERSION_RELEASE);
+		if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
+			if (apple_gmux_is_indexed(res->start))
+				type = APPLE_GMUX_TYPE_INDEXED;
+			else
+				goto out;
+		}
+	} else {
+		res = pnp_get_resource(pnp_dev, IORESOURCE_MEM, 0);
+		if (res && apple_gmux_is_mmio(res->start))
+			type = APPLE_GMUX_TYPE_MMIO;
 		else
 			goto out;
 	}
-- 
2.39.1

Re: [PATCH v3 4/5] apple-gmux: support MMIO gmux on T2 Macs

[Lukas Wunner]

On Sun, Feb 19, 2023 at 12:20:06AM +1100, Orlando Chamberlain wrote:
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -28,15 +28,17 @@
>   * DOC: Overview
>   *
>   * gmux is a microcontroller built into the MacBook Pro to support dual GPUs:
> - * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on retinas.
> + * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on pre-T2 retinas.
> + * The chip used on T2 Macs is not known.

I've just taken a look at the schematics of the MBP16,1 and it turns out
that T2 *is* the gmux.  The chip is called H9M.  The interaction with the
OS is through that MMIO area.

There's a GPIO expander attached to T2 via I2C (NXP PCAL6524), probably
because they didn't have enough GPIO pins available on T2 itself.
The GPIO expander enables/disables the voltage regulators of the discrete
GPU, hence can cut power to it.  It also drives panel power and has a GPIO
to switch the mux.

The mux is an NXP CBTL06142 as on previous models.

Thanks,

Lukas

Re: [PATCH v3 4/5] apple-gmux: support MMIO gmux on T2 Macs

[Orlando Chamberlain]

On Mon, 20 Feb 2023 10:20:48 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> On Sun, Feb 19, 2023 at 12:20:06AM +1100, Orlando Chamberlain wrote:
> > --- a/drivers/platform/x86/apple-gmux.c
> > +++ b/drivers/platform/x86/apple-gmux.c
> > @@ -28,15 +28,17 @@
> >   * DOC: Overview
> >   *
> >   * gmux is a microcontroller built into the MacBook Pro to support
> > dual GPUs:
> > - * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on
> > retinas.
> > + * A `Lattice XP2`_ on pre-retinas, a `Renesas R4F2113`_ on pre-T2
> > retinas.
> > + * The chip used on T2 Macs is not known.  
> 
> I've just taken a look at the schematics of the MBP16,1 and it turns
> out that T2 *is* the gmux.  The chip is called H9M.  The interaction
> with the OS is through that MMIO area.
> 
> There's a GPIO expander attached to T2 via I2C (NXP PCAL6524),
> probably because they didn't have enough GPIO pins available on T2
> itself. The GPIO expander enables/disables the voltage regulators of
> the discrete GPU, hence can cut power to it.  It also drives panel
> power and has a GPIO to switch the mux.
> 
> The mux is an NXP CBTL06142 as on previous models.

Thanks for figuring that all out, I can add that to the documentation.

> 
> Thanks,
> 
> Lukas

[PATCH v3 5/5] apple-gmux: add debugfs interface

[Orlando Chamberlain]

Allow reading and writing gmux ports from userspace.

For example:

echo 4 > /sys/kernel/debug/apple_gmux/selected_port
cat /sys/kernel/debug/apple_gmux/selected_port_data | xxd -p

Will show the gmux version information (00000005 in this case)

Signed-off-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
---
v2->v3: don't check if the folder failed to be created
 drivers/platform/x86/apple-gmux.c | 82 +++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 01e7b1939916..4cbdc9f9bd10 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -22,6 +22,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
+#include <linux/debugfs.h>
 #include <asm/io.h>
 
 /**
@@ -66,6 +67,10 @@ struct apple_gmux_data {
 	enum vga_switcheroo_client_id switch_state_external;
 	enum vga_switcheroo_state power_state;
 	struct completion powerchange_done;
+
+	/* debugfs data */
+	u8 selected_port;
+	struct dentry *debug_dentry;
 };
 
 static struct apple_gmux_data *apple_gmux_data;
@@ -672,6 +677,81 @@ static void gmux_notify_handler(acpi_handle device, u32 value, void *context)
 		complete(&gmux_data->powerchange_done);
 }
 
+/**
+ * DOC: Debugfs Interface
+ *
+ * gmux ports can be accessed from userspace as a debugfs interface. For example:
+ *
+ * # echo 4 > /sys/kernel/debug/apple_gmux/selected_port
+ * # cat /sys/kernel/debug/apple_gmux/selected_port_data | xxd -p
+ * 00000005
+ *
+ * Reads 4 bytes from port 4 (GMUX_PORT_VERSION_MAJOR).
+ *
+ * 1 and 4 byte writes are also allowed.
+ */
+
+static ssize_t gmux_selected_port_data_write(struct file *file,
+		const char __user *userbuf, size_t count, loff_t *ppos)
+{
+	struct apple_gmux_data *gmux_data = file->private_data;
+	int ret;
+
+	if (*ppos)
+		return -EINVAL;
+
+	if (count == 1) {
+		u8 data;
+
+		ret = copy_from_user(&data, userbuf, 1);
+		if (ret)
+			return ret;
+		gmux_write8(gmux_data, gmux_data->selected_port, data);
+	} else if (count == 4) {
+		u32 data;
+
+		ret = copy_from_user(&data, userbuf, 4);
+		if (ret)
+			return ret;
+		gmux_write32(gmux_data, gmux_data->selected_port, data);
+	} else
+		return -EINVAL;
+
+	return count;
+}
+
+static ssize_t gmux_selected_port_data_read(struct file *file,
+		char __user *userbuf, size_t count, loff_t *ppos)
+{
+	struct apple_gmux_data *gmux_data = file->private_data;
+	u32 data;
+
+	data = gmux_read32(gmux_data, gmux_data->selected_port);
+
+	return simple_read_from_buffer(userbuf, count, ppos, &data, sizeof(data));
+}
+
+static const struct file_operations gmux_port_data_ops = {
+	.open = simple_open,
+	.write = gmux_selected_port_data_write,
+	.read = gmux_selected_port_data_read
+};
+
+static void gmux_init_debugfs(struct apple_gmux_data *gmux_data)
+{
+	gmux_data->debug_dentry = debugfs_create_dir(KBUILD_MODNAME, NULL);
+
+	debugfs_create_u8("selected_port", 0644, gmux_data->debug_dentry,
+			&gmux_data->selected_port);
+	debugfs_create_file("selected_port_data", 0644, gmux_data->debug_dentry,
+			gmux_data, &gmux_port_data_ops);
+}
+
+static void gmux_fini_debugfs(struct apple_gmux_data *gmux_data)
+{
+	debugfs_remove_recursive(gmux_data->debug_dentry);
+}
+
 static int gmux_suspend(struct device *dev)
 {
 	struct pnp_dev *pnp = to_pnp_dev(dev);
@@ -872,6 +952,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		goto err_register_handler;
 	}
 
+	gmux_init_debugfs(gmux_data);
 	return 0;
 
 err_register_handler:
@@ -903,6 +984,7 @@ static void gmux_remove(struct pnp_dev *pnp)
 {
 	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
 
+	gmux_fini_debugfs(gmux_data);
 	vga_switcheroo_unregister_handler();
 	gmux_disable_interrupts(gmux_data);
 	if (gmux_data->gpe >= 0) {
-- 
2.39.1

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Hans de Goede]

Hi,

On 2/18/23 14:20, Orlando Chamberlain wrote:
> Hi All,
> 
> This patch series adds support for the MMIO based gmux present on these
> Dual GPU Apple T2 Macs: MacBookPro15,1, MacBookPro15,3, MacBookPro16,1,
> MacBookPro16,4 (although amdgpu isn't working on MacBookPro16,4 [1]).
> 
> Changes from v2[2]:
> 
> - Add "," to last item in apple_gmux_type enum
> - Don't not clear interrupts when status is 0
> - Don't check if we failed to make debugfs folder
> - Check for fake mmio gmux

Thanks, this looks good to me now:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the entire series.

Can you prepare a v4 addressing Lukas's remarks:

1. Switch to acpi_execute_simple_method()
2. Add extra info about the gmux provided by Lukas 
   to either the docs or as comments

With those changes added I believe v4 will be ready
for merging.

Regards,

Hans


> 
> # 1:
> 
> has a slight change in how the switch state is read: instead of checking
> for x == 2, check !(x & 1)
> 
> # 2:
> 
> implements a system to support more than 2 gmux types
> 
> # 3:
> 
> start using the gmux's GMSP acpi method when handling interrupts on MMIO
> gmux's. This is needed for the MMIO gmux's to clear interrupts.
> 
> # 4:
> 
> Adds support for the MMIO based gmux on T2 macs.
> 
> # 5:
> 
> Add a debugfs interface to apple-gmux so data from ports can be read
> and written to from userspace.
> 
> This can be used for more easily researching what unknown ports do,
> and switching gpus when vga_switcheroo isn't ready (e.g. when one gpu
> is bound to vfio-pci and in use by a Windows VM, I can use this to
> switch my internal display between Linux and Windows easily).
> 
> # Issues:
> 
> 1. Switching gpus at runtime has the same issue as indexed gmux's: the
> inactive gpu can't probe the DDC lines for eDP [3]
> 
> 2. iMacPro1,1, iMac20,1 and iMac20,2 all seem to have a gmux in their
> acpi tables, but they shouldn't. A check that hopefully will detect this
> is used, but it's untested as I don't have any of those computers.
> 
> 3. Powering on the amdgpu with vga_switcheroo doesn't work well. I'm
> told on the MacBookPro15,1 it works sometimes, and adding delays helps,
> but on my MacBookPro16,1 I haven't been able to get it to work at all:
> 
> amdgpu: switched off
> amdgpu: switched on
> amdgpu 0000:03:00.0:
>     Unable to change power state from D3hot to D0, device inaccessible
> amdgpu 0000:03:00.0:
>     Unable to change power state from D3cold to D0, device inaccessible
> [drm] PCIE GART of 512M enabled (table at 0x00000080FEE00000).
> [drm] PSP is resuming...
> [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
> [drm:psp_resume [amdgpu]] *ERROR* PSP resume failed
> [drm:amdgpu_device_fw_loading [amdgpu]]
>     *ERROR* resume of IP block <psp> failed -62
> amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_resume failed (-62).
> snd_hda_intel 0000:03:00.1: Enabling via vga_switcheroo
> snd_hda_intel 0000:03:00.1:
>     Unable to change power state from D3cold to D0, device inaccessible
> snd_hda_intel 0000:03:00.1: CORB reset timeout#2, CORBRP = 65535
> snd_hda_codec_hdmi hdaudioC0D0: Unable to sync register 0x2f0d00. -5
> 
> There are some acpi methods (PWRD, PWG1 [4, 5]) that macOS calls when
> changing the amdgpu's power state, but we don't use them and that could be
> a cause. Additionally unlike previous generation Macbooks which work
> better, on MacBookPro16,1 the gpu is located behind 2 pci bridges:
> 
> 01:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>     Navi 10 XL Upstream Port of PCI Express Switch (rev 43)
> 02:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>     Navi 10 XL Downstream Port of PCI Express Switch
> 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
>     Navi 14 [Radeon RX 5500/5500M / Pro 5500M] (rev 43)
> 03:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI]
>     Navi 10 HDMI Audio
> 
> Upon attempting to power on the gpu with vga_switcheroo, all these
> devices except 01:00.0 have their config space filled with 1s.
> Rescanning pci makes the config space of all the devices go back to
> normal, however amdgpu still fails to resume with the same logs as
> above.
> 
> [1]: https://lore.kernel.org/all/3AFB9142-2BD0-46F9-AEA9-C9C5D13E68E6@live.com/
> [2]: https://lore.kernel.org/platform-driver-x86/20230216122342.5918-1-orlandoch.dev@gmail.com/
> [3]: https://lore.kernel.org/all/9eed8ede6f15a254ad578e783b050e1c585d5a15.1439288957.git.lukas@wunner.de/
> [4]: https://gist.github.com/Redecorating/6c7136b7a4ac7ce3b77d8e41740dd87b
> [5]: https://lore.kernel.org/all/20120710160555.GA31562@srcf.ucam.org/
> 
> Orlando Chamberlain (5):
>   apple-gmux: use first bit to check switch state
>   apple-gmux: refactor gmux types
>   apple-gmux: Use GMSP acpi method for interrupt clear
>   apple-gmux: support MMIO gmux on T2 Macs
>   apple-gmux: add debugfs interface
> 
>  drivers/platform/x86/apple-gmux.c | 349 ++++++++++++++++++++++++++----
>  include/linux/apple-gmux.h        |  70 ++++--
>  2 files changed, 357 insertions(+), 62 deletions(-)
> 

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Orlando Chamberlain]

Hi Hans,

On Wed, 1 Mar 2023 13:54:37 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 2/18/23 14:20, Orlando Chamberlain wrote:
> > Hi All,
> > 
> > This patch series adds support for the MMIO based gmux present on
> > these Dual GPU Apple T2 Macs: MacBookPro15,1, MacBookPro15,3,
> > MacBookPro16,1, MacBookPro16,4 (although amdgpu isn't working on
> > MacBookPro16,4 [1]).
> > 
> > Changes from v2[2]:
> > 
> > - Add "," to last item in apple_gmux_type enum
> > - Don't not clear interrupts when status is 0
> > - Don't check if we failed to make debugfs folder
> > - Check for fake mmio gmux  
> 
> Thanks, this looks good to me now:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> for the entire series.
> 
> Can you prepare a v4 addressing Lukas's remarks:
> 
> 1. Switch to acpi_execute_simple_method()
> 2. Add extra info about the gmux provided by Lukas 
>    to either the docs or as comments
> 
> With those changes added I believe v4 will be ready
> for merging.

Good to hear, I'll try to send out a v4 tonight.

> 
> Regards,
> 
> Hans
> 
> 
> > 
> > # 1:
> > 
> > has a slight change in how the switch state is read: instead of
> > checking for x == 2, check !(x & 1)
> > 
> > # 2:
> > 
> > implements a system to support more than 2 gmux types
> > 
> > # 3:
> > 
> > start using the gmux's GMSP acpi method when handling interrupts on
> > MMIO gmux's. This is needed for the MMIO gmux's to clear interrupts.
> > 
> > # 4:
> > 
> > Adds support for the MMIO based gmux on T2 macs.
> > 
> > # 5:
> > 
> > Add a debugfs interface to apple-gmux so data from ports can be read
> > and written to from userspace.
> > 
> > This can be used for more easily researching what unknown ports do,
> > and switching gpus when vga_switcheroo isn't ready (e.g. when one
> > gpu is bound to vfio-pci and in use by a Windows VM, I can use this
> > to switch my internal display between Linux and Windows easily).
> > 
> > # Issues:
> > 
> > 1. Switching gpus at runtime has the same issue as indexed gmux's:
> > the inactive gpu can't probe the DDC lines for eDP [3]
> > 
> > 2. iMacPro1,1, iMac20,1 and iMac20,2 all seem to have a gmux in
> > their acpi tables, but they shouldn't. A check that hopefully will
> > detect this is used, but it's untested as I don't have any of those
> > computers.
> > 
> > 3. Powering on the amdgpu with vga_switcheroo doesn't work well. I'm
> > told on the MacBookPro15,1 it works sometimes, and adding delays
> > helps, but on my MacBookPro16,1 I haven't been able to get it to
> > work at all:
> > 
> > amdgpu: switched off
> > amdgpu: switched on
> > amdgpu 0000:03:00.0:
> >     Unable to change power state from D3hot to D0, device
> > inaccessible amdgpu 0000:03:00.0:
> >     Unable to change power state from D3cold to D0, device
> > inaccessible [drm] PCIE GART of 512M enabled (table at
> > 0x00000080FEE00000). [drm] PSP is resuming...
> > [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
> > [drm:psp_resume [amdgpu]] *ERROR* PSP resume failed
> > [drm:amdgpu_device_fw_loading [amdgpu]]
> >     *ERROR* resume of IP block <psp> failed -62
> > amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_resume failed (-62).
> > snd_hda_intel 0000:03:00.1: Enabling via vga_switcheroo
> > snd_hda_intel 0000:03:00.1:
> >     Unable to change power state from D3cold to D0, device
> > inaccessible snd_hda_intel 0000:03:00.1: CORB reset timeout#2,
> > CORBRP = 65535 snd_hda_codec_hdmi hdaudioC0D0: Unable to sync
> > register 0x2f0d00. -5
> > 
> > There are some acpi methods (PWRD, PWG1 [4, 5]) that macOS calls
> > when changing the amdgpu's power state, but we don't use them and
> > that could be a cause. Additionally unlike previous generation
> > Macbooks which work better, on MacBookPro16,1 the gpu is located
> > behind 2 pci bridges:
> > 
> > 01:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
> >     Navi 10 XL Upstream Port of PCI Express Switch (rev 43)
> > 02:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
> >     Navi 10 XL Downstream Port of PCI Express Switch
> > 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc.
> > [AMD/ATI] Navi 14 [Radeon RX 5500/5500M / Pro 5500M] (rev 43)
> > 03:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI]
> >     Navi 10 HDMI Audio
> > 
> > Upon attempting to power on the gpu with vga_switcheroo, all these
> > devices except 01:00.0 have their config space filled with 1s.
> > Rescanning pci makes the config space of all the devices go back to
> > normal, however amdgpu still fails to resume with the same logs as
> > above.
> > 
> > [1]:
> > https://lore.kernel.org/all/3AFB9142-2BD0-46F9-AEA9-C9C5D13E68E6@live.com/
> > [2]:
> > https://lore.kernel.org/platform-driver-x86/20230216122342.5918-1-orlandoch.dev@gmail.com/
> > [3]:
> > https://lore.kernel.org/all/9eed8ede6f15a254ad578e783b050e1c585d5a15.1439288957.git.lukas@wunner.de/
> > [4]:
> > https://gist.github.com/Redecorating/6c7136b7a4ac7ce3b77d8e41740dd87b
> > [5]:
> > https://lore.kernel.org/all/20120710160555.GA31562@srcf.ucam.org/
> > 
> > Orlando Chamberlain (5):
> >   apple-gmux: use first bit to check switch state
> >   apple-gmux: refactor gmux types
> >   apple-gmux: Use GMSP acpi method for interrupt clear
> >   apple-gmux: support MMIO gmux on T2 Macs
> >   apple-gmux: add debugfs interface
> > 
> >  drivers/platform/x86/apple-gmux.c | 349
> > ++++++++++++++++++++++++++---- include/linux/apple-gmux.h        |
> > 70 ++++-- 2 files changed, 357 insertions(+), 62 deletions(-)
> >   
> 

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Aditya Garg]

> On 18-Feb-2023, at 6:50 PM, Orlando Chamberlain <orlandoch.dev@gmail.com> wrote:
> Hi All,
> This patch series adds support for the MMIO based gmux present on these
> Dual GPU Apple T2 Macs: MacBookPro15,1, MacBookPro15,3, MacBookPro16,1,
> MacBookPro16,4 (although amdgpu isn't working on MacBookPro16,4 [1]).
> 

Could be an upstream bug, but I’ve noticed that after using these patches, if I add `acpi_backlight=video` in the command line, it still uses `gmux_backlight`

Hans, any ideas why?

> Changes from v2[2]:
> 
> - Add "," to last item in apple_gmux_type enum
> - Don't not clear interrupts when status is 0
> - Don't check if we failed to make debugfs folder
> - Check for fake mmio gmux
> 
> # 1:
> 
> has a slight change in how the switch state is read: instead of checking
> for x == 2, check !(x & 1)
> 
> # 2:
> 
> implements a system to support more than 2 gmux types
> 
> # 3:
> 
> start using the gmux's GMSP acpi method when handling interrupts on MMIO
> gmux's. This is needed for the MMIO gmux's to clear interrupts.
> 
> # 4:
> 
> Adds support for the MMIO based gmux on T2 macs.
> 
> # 5:
> 
> Add a debugfs interface to apple-gmux so data from ports can be read
> and written to from userspace.
> 
> This can be used for more easily researching what unknown ports do,
> and switching gpus when vga_switcheroo isn't ready (e.g. when one gpu
> is bound to vfio-pci and in use by a Windows VM, I can use this to
> switch my internal display between Linux and Windows easily).
> 
> # Issues:
> 
> 1. Switching gpus at runtime has the same issue as indexed gmux's: the
> inactive gpu can't probe the DDC lines for eDP [3]
> 
> 2. iMacPro1,1, iMac20,1 and iMac20,2 all seem to have a gmux in their
> acpi tables, but they shouldn't. A check that hopefully will detect this
> is used, but it's untested as I don't have any of those computers.
> 
> 3. Powering on the amdgpu with vga_switcheroo doesn't work well. I'm
> told on the MacBookPro15,1 it works sometimes, and adding delays helps,
> but on my MacBookPro16,1 I haven't been able to get it to work at all:
> 
> amdgpu: switched off
> amdgpu: switched on
> amdgpu 0000:03:00.0:
>    Unable to change power state from D3hot to D0, device inaccessible
> amdgpu 0000:03:00.0:
>    Unable to change power state from D3cold to D0, device inaccessible
> [drm] PCIE GART of 512M enabled (table at 0x00000080FEE00000).
> [drm] PSP is resuming...
> [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
> [drm:psp_resume [amdgpu]] *ERROR* PSP resume failed
> [drm:amdgpu_device_fw_loading [amdgpu]]
>    *ERROR* resume of IP block <psp> failed -62
> amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_resume failed (-62).
> snd_hda_intel 0000:03:00.1: Enabling via vga_switcheroo
> snd_hda_intel 0000:03:00.1:
>    Unable to change power state from D3cold to D0, device inaccessible
> snd_hda_intel 0000:03:00.1: CORB reset timeout#2, CORBRP = 65535
> snd_hda_codec_hdmi hdaudioC0D0: Unable to sync register 0x2f0d00. -5
> 
> There are some acpi methods (PWRD, PWG1 [4, 5]) that macOS calls when
> changing the amdgpu's power state, but we don't use them and that could be
> a cause. Additionally unlike previous generation Macbooks which work
> better, on MacBookPro16,1 the gpu is located behind 2 pci bridges:
> 
> 01:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>    Navi 10 XL Upstream Port of PCI Express Switch (rev 43)
> 02:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>    Navi 10 XL Downstream Port of PCI Express Switch
> 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
>    Navi 14 [Radeon RX 5500/5500M / Pro 5500M] (rev 43)
> 03:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI]
>    Navi 10 HDMI Audio
> 
> Upon attempting to power on the gpu with vga_switcheroo, all these
> devices except 01:00.0 have their config space filled with 1s.
> Rescanning pci makes the config space of all the devices go back to
> normal, however amdgpu still fails to resume with the same logs as
> above.
> 
> [1]: https://lore.kernel.org/all/3AFB9142-2BD0-46F9-AEA9-C9C5D13E68E6@live.com/
> [2]: https://lore.kernel.org/platform-driver-x86/20230216122342.5918-1-orlandoch.dev@gmail.com/
> [3]: https://lore.kernel.org/all/9eed8ede6f15a254ad578e783b050e1c585d5a15.1439288957.git.lukas@wunner.de/
> [4]: https://gist.github.com/Redecorating/6c7136b7a4ac7ce3b77d8e41740dd87b
> [5]: https://lore.kernel.org/all/20120710160555.GA31562@srcf.ucam.org/
> 
> Orlando Chamberlain (5):
>  apple-gmux: use first bit to check switch state
>  apple-gmux: refactor gmux types
>  apple-gmux: Use GMSP acpi method for interrupt clear
>  apple-gmux: support MMIO gmux on T2 Macs
>  apple-gmux: add debugfs interface
> 
> drivers/platform/x86/apple-gmux.c | 349 ++++++++++++++++++++++++++----
> include/linux/apple-gmux.h        |  70 ++++--
> 2 files changed, 357 insertions(+), 62 deletions(-)
> 
> -- 
> 2.39.1

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 5343 bytes --]

Hi,

On 2/21/23 10:24, Aditya Garg wrote:
> 
>> On 18-Feb-2023, at 6:50 PM, Orlando Chamberlain <orlandoch.dev@gmail.com> wrote:
>> Hi All,
>> This patch series adds support for the MMIO based gmux present on these
>> Dual GPU Apple T2 Macs: MacBookPro15,1, MacBookPro15,3, MacBookPro16,1,
>> MacBookPro16,4 (although amdgpu isn't working on MacBookPro16,4 [1]).
>>
> 
> Could be an upstream bug, but I’ve noticed that after using these patches, if I add `acpi_backlight=video` in the command line, it still uses `gmux_backlight`
> 
> Hans, any ideas why?

Currently the backlight registration in apple-gmux.c is unconditional
(if a GMUX is detected).

I have attached a patch to make it honor the acpi_backlight=xxx
kernel commandline option like most other x86/ACPI backlight drivers
do, please give this a test.

Regards,

Hans


> 
>> Changes from v2[2]:
>>
>> - Add "," to last item in apple_gmux_type enum
>> - Don't not clear interrupts when status is 0
>> - Don't check if we failed to make debugfs folder
>> - Check for fake mmio gmux
>>
>> # 1:
>>
>> has a slight change in how the switch state is read: instead of checking
>> for x == 2, check !(x & 1)
>>
>> # 2:
>>
>> implements a system to support more than 2 gmux types
>>
>> # 3:
>>
>> start using the gmux's GMSP acpi method when handling interrupts on MMIO
>> gmux's. This is needed for the MMIO gmux's to clear interrupts.
>>
>> # 4:
>>
>> Adds support for the MMIO based gmux on T2 macs.
>>
>> # 5:
>>
>> Add a debugfs interface to apple-gmux so data from ports can be read
>> and written to from userspace.
>>
>> This can be used for more easily researching what unknown ports do,
>> and switching gpus when vga_switcheroo isn't ready (e.g. when one gpu
>> is bound to vfio-pci and in use by a Windows VM, I can use this to
>> switch my internal display between Linux and Windows easily).
>>
>> # Issues:
>>
>> 1. Switching gpus at runtime has the same issue as indexed gmux's: the
>> inactive gpu can't probe the DDC lines for eDP [3]
>>
>> 2. iMacPro1,1, iMac20,1 and iMac20,2 all seem to have a gmux in their
>> acpi tables, but they shouldn't. A check that hopefully will detect this
>> is used, but it's untested as I don't have any of those computers.
>>
>> 3. Powering on the amdgpu with vga_switcheroo doesn't work well. I'm
>> told on the MacBookPro15,1 it works sometimes, and adding delays helps,
>> but on my MacBookPro16,1 I haven't been able to get it to work at all:
>>
>> amdgpu: switched off
>> amdgpu: switched on
>> amdgpu 0000:03:00.0:
>>    Unable to change power state from D3hot to D0, device inaccessible
>> amdgpu 0000:03:00.0:
>>    Unable to change power state from D3cold to D0, device inaccessible
>> [drm] PCIE GART of 512M enabled (table at 0x00000080FEE00000).
>> [drm] PSP is resuming...
>> [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
>> [drm:psp_resume [amdgpu]] *ERROR* PSP resume failed
>> [drm:amdgpu_device_fw_loading [amdgpu]]
>>    *ERROR* resume of IP block <psp> failed -62
>> amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_resume failed (-62).
>> snd_hda_intel 0000:03:00.1: Enabling via vga_switcheroo
>> snd_hda_intel 0000:03:00.1:
>>    Unable to change power state from D3cold to D0, device inaccessible
>> snd_hda_intel 0000:03:00.1: CORB reset timeout#2, CORBRP = 65535
>> snd_hda_codec_hdmi hdaudioC0D0: Unable to sync register 0x2f0d00. -5
>>
>> There are some acpi methods (PWRD, PWG1 [4, 5]) that macOS calls when
>> changing the amdgpu's power state, but we don't use them and that could be
>> a cause. Additionally unlike previous generation Macbooks which work
>> better, on MacBookPro16,1 the gpu is located behind 2 pci bridges:
>>
>> 01:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>>    Navi 10 XL Upstream Port of PCI Express Switch (rev 43)
>> 02:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD/ATI]
>>    Navi 10 XL Downstream Port of PCI Express Switch
>> 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
>>    Navi 14 [Radeon RX 5500/5500M / Pro 5500M] (rev 43)
>> 03:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI]
>>    Navi 10 HDMI Audio
>>
>> Upon attempting to power on the gpu with vga_switcheroo, all these
>> devices except 01:00.0 have their config space filled with 1s.
>> Rescanning pci makes the config space of all the devices go back to
>> normal, however amdgpu still fails to resume with the same logs as
>> above.
>>
>> [1]: https://lore.kernel.org/all/3AFB9142-2BD0-46F9-AEA9-C9C5D13E68E6@live.com/
>> [2]: https://lore.kernel.org/platform-driver-x86/20230216122342.5918-1-orlandoch.dev@gmail.com/
>> [3]: https://lore.kernel.org/all/9eed8ede6f15a254ad578e783b050e1c585d5a15.1439288957.git.lukas@wunner.de/
>> [4]: https://gist.github.com/Redecorating/6c7136b7a4ac7ce3b77d8e41740dd87b
>> [5]: https://lore.kernel.org/all/20120710160555.GA31562@srcf.ucam.org/
>>
>> Orlando Chamberlain (5):
>>  apple-gmux: use first bit to check switch state
>>  apple-gmux: refactor gmux types
>>  apple-gmux: Use GMSP acpi method for interrupt clear
>>  apple-gmux: support MMIO gmux on T2 Macs
>>  apple-gmux: add debugfs interface
>>
>> drivers/platform/x86/apple-gmux.c | 349 ++++++++++++++++++++++++++----
>> include/linux/apple-gmux.h        |  70 ++++--
>> 2 files changed, 357 insertions(+), 62 deletions(-)
>>
>> -- 
>> 2.39.1

[-- Attachment #2: 0001-platform-x86-apple-gmux-Add-acpi_video_get_backlight.patch --]
[-- Type: text/x-patch, Size: 3580 bytes --]

From 97228b107fe075b94c59a5b68183e00fe7367d15 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 21 Feb 2023 13:26:19 +0100
Subject: [PATCH] platform/x86: apple-gmux: Add acpi_video_get_backlight_type()
 check

Make apple-gmux backlight registration honor the acpi_backlight=...
kernel commandline option which is used to select the backlight
control method on x86/ACPI devices.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/apple-gmux.c | 59 +++++++++++++++++--------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 9333f82cfa8a..246caaab27aa 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -21,6 +21,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
+#include <acpi/video.h>
 #include <asm/io.h>
 
 /**
@@ -562,6 +563,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	struct backlight_properties props;
 	struct backlight_device *bdev;
 	u8 ver_major, ver_minor, ver_release;
+	bool register_bdev = true;
 	int ret = -ENXIO;
 	acpi_status status;
 	unsigned long long gpe;
@@ -610,33 +612,38 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 	props.type = BACKLIGHT_PLATFORM;
 	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
 
-	/*
-	 * Currently it's assumed that the maximum brightness is less than
-	 * 2^24 for compatibility with old gmux versions. Cap the max
-	 * brightness at this value, but print a warning if the hardware
-	 * reports something higher so that it can be fixed.
-	 */
-	if (WARN_ON(props.max_brightness > GMUX_MAX_BRIGHTNESS))
-		props.max_brightness = GMUX_MAX_BRIGHTNESS;
-
-	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
-					 gmux_data, &gmux_bl_ops, &props);
-	if (IS_ERR(bdev)) {
-		ret = PTR_ERR(bdev);
-		goto err_release;
-	}
-
-	gmux_data->bdev = bdev;
-	bdev->props.brightness = gmux_get_brightness(bdev);
-	backlight_update_status(bdev);
+#if IS_REACHABLE(CONFIG_ACPI_VIDEO)
+	register_bdev = acpi_video_get_backlight_type() == acpi_backlight_apple_gmux;
+#endif
+	if (register_bdev) {
+		/*
+		 * Currently it's assumed that the maximum brightness is less than
+		 * 2^24 for compatibility with old gmux versions. Cap the max
+		 * brightness at this value, but print a warning if the hardware
+		 * reports something higher so that it can be fixed.
+		 */
+		if (WARN_ON(props.max_brightness > GMUX_MAX_BRIGHTNESS))
+			props.max_brightness = GMUX_MAX_BRIGHTNESS;
+
+		bdev = backlight_device_register("gmux_backlight", &pnp->dev,
+						 gmux_data, &gmux_bl_ops, &props);
+		if (IS_ERR(bdev)) {
+			ret = PTR_ERR(bdev);
+			goto err_release;
+		}
 
-	/*
-	 * The backlight situation on Macs is complicated. If the gmux is
-	 * present it's the best choice, because it always works for
-	 * backlight control and supports more levels than other options.
-	 * Disable the other backlight choices.
-	 */
-	apple_bl_unregister();
+		gmux_data->bdev = bdev;
+		bdev->props.brightness = gmux_get_brightness(bdev);
+		backlight_update_status(bdev);
+
+		/*
+		 * The backlight situation on Macs is complicated. If the gmux is
+		 * present it's the best choice, because it always works for
+		 * backlight control and supports more levels than other options.
+		 * Disable the other backlight choices.
+		 */
+		apple_bl_unregister();
+	}
 
 	gmux_data->power_state = VGA_SWITCHEROO_ON;
 
-- 
2.39.1

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Aditya Garg]

> Currently the backlight registration in apple-gmux.c is unconditional
> (if a GMUX is detected).
> 
> I have attached a patch to make it honor the acpi_backlight=xxx
> kernel commandline option like most other x86/ACPI backlight drivers
> do, please give this a test.
> 
> Regards,
> 
> Hans
> 

Also, another issue, I have pluymouth enabled with FRAMEBUFFER=y. After using the gmux patches, the size of the theme has become too tiny for an initial few seconds, then it gets normal. I tested this by compiling a kernel without these patches, and the bug got fixed. I also tried using acpi backlight using command line (that made me discover the `acpi_backlight=video` not working bug). Is that also related to some upstream bug?

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Aditya Garg]

> 
> Also, another issue, I have pluymouth enabled with FRAMEBUFFER=y. After using the gmux patches, the size of the theme has become too tiny for an initial few seconds, then it gets normal. I tested this by compiling a kernel without these patches, and the bug got fixed. I also tried using acpi backlight using command line (that made me discover the `acpi_backlight=video` not working bug). Is that also related to some upstream bug?

Hi Hans

It turns out that Apple-gmux had to be loaded in the initramfs for Plymouth to work properly. So, it actually was my mistake.

Also I noticed that the respond to your patch for me to test didn’t reach since it was an HTML email. So, yes, your patch honours `acpi_backlight=video`

Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs

[Hans de Goede]

Hi,

On 2/21/23 20:07, Aditya Garg wrote:
> Hi
> 
>> Currently the backlight registration in apple-gmux.c is unconditional
>> (if a GMUX is detected).
>>
>> I have attached a patch to make it honor the acpi_backlight=xxx
>> kernel commandline option like most other x86/ACPI backlight drivers
>> do, please give this a test.
>>
>> Regards,
>>
>> Hans
> 
> I had to modify the patch a bit to make it apply alongwith the gmux patches by Orlando, and the bug seems to be fixed now.
> Attaching the patch I applied with this email.

Yes I didn't have Orlando's patches in my tree when preparing this.

Thank you for testing this. I'll rebase the patch on top of Orlando's
patches once those have been merged and then officially submit
it for review + merging.

Regards,

Hans