by Aditya Garg

[permalink] [raw]

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

> On 18-Feb-2023, at 6:50 PM, Orlando Chamberlain <[email protected]> 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/[email protected]/
> [2]: https://lore.kernel.org/platform-driver-x86/[email protected]/
> [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/[email protected]/
>
> 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

2023-02-21 12:32:34

Hi Hans,

On Wed, 1 Mar 2023 13:54:37 +0100
Hans de Goede <[email protected]> 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 <[email protected]>
>
> 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/[email protected]/
> > [2]:
> > https://lore.kernel.org/platform-driver-x86/[email protected]/
> > [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/[email protected]/
> >
> > 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(-)
> >
>