All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
@ 2023-02-21  9:24 Aditya Garg
  2023-02-21 12:31 ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Aditya Garg @ 2023-02-21  9:24 UTC (permalink / raw)
  To: Orlando Chamberlain
  Cc: Hans de Goede, Mark Gross, platform-driver-x86, linux-kernel,
	Lukas Wunner, Seth Forshee, Aun-Ali Zaidi, Kerem Karabay


> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
  2023-02-21  9:24 [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs Aditya Garg
@ 2023-02-21 12:31 ` Hans de Goede
  2023-02-21 19:13   ` Aditya Garg
       [not found]   ` <BM1PR01MB093194DDA465CE8D9F081A05B8A59@BM1PR01MB0931.INDPRD01.PROD.OUTLOOK.COM>
  0 siblings, 2 replies; 8+ messages in thread
From: Hans de Goede @ 2023-02-21 12:31 UTC (permalink / raw)
  To: Aditya Garg, Orlando Chamberlain
  Cc: Mark Gross, platform-driver-x86, linux-kernel, Lukas Wunner,
	Seth Forshee, Aun-Ali Zaidi, Kerem Karabay

[-- 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
  2023-02-21 12:31 ` Hans de Goede
@ 2023-02-21 19:13   ` Aditya Garg
  2023-02-22 18:13     ` Aditya Garg
       [not found]   ` <BM1PR01MB093194DDA465CE8D9F081A05B8A59@BM1PR01MB0931.INDPRD01.PROD.OUTLOOK.COM>
  1 sibling, 1 reply; 8+ messages in thread
From: Aditya Garg @ 2023-02-21 19:13 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Orlando Chamberlain, Mark Gross, platform-driver-x86,
	linux-kernel, Lukas Wunner, Seth Forshee, Aun-Ali Zaidi,
	Kerem Karabay


> 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?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
  2023-02-21 19:13   ` Aditya Garg
@ 2023-02-22 18:13     ` Aditya Garg
  0 siblings, 0 replies; 8+ messages in thread
From: Aditya Garg @ 2023-02-22 18:13 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Orlando Chamberlain, Mark Gross, platform-driver-x86,
	linux-kernel, Lukas Wunner, Seth Forshee, Aun-Ali Zaidi,
	Kerem Karabay

> 
> 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`

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
       [not found]   ` <BM1PR01MB093194DDA465CE8D9F081A05B8A59@BM1PR01MB0931.INDPRD01.PROD.OUTLOOK.COM>
@ 2023-03-01 12:56     ` Hans de Goede
  0 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2023-03-01 12:56 UTC (permalink / raw)
  To: Aditya Garg
  Cc: Orlando Chamberlain, Mark Gross, platform-driver-x86,
	linux-kernel, Lukas Wunner, Seth Forshee, Aun-Ali Zaidi,
	Kerem Karabay

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
  2023-03-01 12:54 ` Hans de Goede
@ 2023-03-03  7:54   ` Orlando Chamberlain
  0 siblings, 0 replies; 8+ messages in thread
From: Orlando Chamberlain @ 2023-03-03  7:54 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, platform-driver-x86, linux-kernel, Lukas Wunner,
	Seth Forshee, Aditya Garg, Aun-Ali Zaidi, Kerem Karabay

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(-)
> >   
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
  2023-02-18 13:20 Orlando Chamberlain
@ 2023-03-01 12:54 ` Hans de Goede
  2023-03-03  7:54   ` Orlando Chamberlain
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2023-03-01 12:54 UTC (permalink / raw)
  To: Orlando Chamberlain
  Cc: Mark Gross, platform-driver-x86, linux-kernel, Lukas Wunner,
	Seth Forshee, Aditya Garg, Aun-Ali Zaidi, Kerem Karabay

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(-)
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs
@ 2023-02-18 13:20 Orlando Chamberlain
  2023-03-01 12:54 ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Orlando Chamberlain @ 2023-02-18 13:20 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, platform-driver-x86, linux-kernel, Lukas Wunner,
	Seth Forshee, Aditya Garg, Aun-Ali Zaidi, Kerem Karabay,
	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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-03-03  7:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-21  9:24 [PATCH v3 0/5] apple-gmux: support MMIO gmux type on T2 Macs Aditya Garg
2023-02-21 12:31 ` Hans de Goede
2023-02-21 19:13   ` Aditya Garg
2023-02-22 18:13     ` Aditya Garg
     [not found]   ` <BM1PR01MB093194DDA465CE8D9F081A05B8A59@BM1PR01MB0931.INDPRD01.PROD.OUTLOOK.COM>
2023-03-01 12:56     ` Hans de Goede
  -- strict thread matches above, loose matches on Subject: below --
2023-02-18 13:20 Orlando Chamberlain
2023-03-01 12:54 ` Hans de Goede
2023-03-03  7:54   ` Orlando Chamberlain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.