| Message ID | 20190212220230.1568-1-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pci/quirks: Add quirk to reset nvgpu at boot for the Lenovo ThinkPad P50 | expand |
Hi Lyude, On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > On a very specific subset of ThinkPad P50 SKUs, particularly ones that > come with a Quadro M1000M chip instead of the M2000M variant, the BIOS > seems to have a very nasty habit of not always resetting the secondary > Nvidia GPU between full reboots if the laptop is configured in Hybrid > Graphics mode. The reason for this happening is unknown, but the > following steps and possibly a good bit of patience will reproduce the > issue: > > 1. Boot up the laptop normally in Hybrid graphics mode > 2. Make sure nouveau is loaded and that the GPU is awake > 2. Allow the nvidia GPU to runtime suspend itself after being idle > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > 4. If nouveau loads up properly, reboot the machine again and go back to > step 2 until you reproduce the issue > > This results in some very strange behavior: the GPU will > quite literally be left in exactly the same state it was in when the > previously booted kernel started the reboot. This has all sorts of bad > sideaffects: for starters, this completely breaks nouveau starting with a > mysterious EVO channel failure that happens well before we've actually > used the EVO channel for anything: There are a lot of moving parts here that are probably obvious to you but not to me. I need help untangling this a bit so I'm comfortable that we got to the root cause and that we're doing something logical as opposed to something that just happens to make things work. I really don't know enough to even ask the right questions... Is there a bug report for this? Bugzilla.kernel.org would be ideal, including "lspci -vvxxx" and dmidecode for the system. Is this running a current BIOS? The date in your log below looks pretty recent, so I assume it is current. I assume "hybrid graphics" means you have two GPUs. Do you select hybrid graphics mode in the BIOS? I assume when you say the Nvidia GPU doesn't get reset on a full reboot, you're talking about a "warm reboot", and that if you actually remove the power and do a cold reboot, there's no problem? I assume Nvidia GPU being active means you are using the performance GPU. Does that mean the integrated GPU is completely unused and Linux does nothing at all with it? Is Linux doing any switching between them? If so, how? I am not 100% confident in the code I've seen that does the switching. > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > 00000002 > > Later on, this causes us to timeout trying to bring up the GR ctx: > > ------------[ cut here ]------------ > nouveau 0000:01:00.0: timeout > WARNING: CPU: 0 PID: 12 at > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > gf100_grctx_generate+0x7b2/0x850 [nouveau] > Modules linked in: nouveau mxm_wmi i915 crc32c_intel ttm i2c_algo_bit > serio_raw drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops > xhci_pci drm xhci_hcd i2c_core wmi video > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc5Lyude-Test+ #29 > Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) > 12/18/2018 > Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] > RIP: 0010:gf100_grctx_generate+0x7b2/0x850 [nouveau] > Code: 85 d2 75 04 48 8b 57 10 48 89 95 28 ff ff ff e8 b4 37 0e e1 48 8b > 95 28 ff ff ff 48 c7 c7 b1 97 57 a0 48 89 c6 e8 5a 38 c0 e0 <0f> 0b e9 > b9 fd ff ff 48 8b 85 60 ff ff ff 48 8b 40 10 48 8b 78 10 > RSP: 0018:ffffc900000b77f0 EFLAGS: 00010286 > RAX: 0000000000000000 RBX: ffff888871af8000 RCX: 0000000000000000 > RDX: ffff88887f41dfe0 RSI: ffff88887f415698 RDI: ffff88887f415698 > RBP: ffffc900000b78c8 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000000 R12: ffff888872118000 > R13: 0000000000000000 R14: ffffffffa0551420 R15: ffffc900000b7818 > FS: 0000000000000000(0000) GS:ffff88887f400000(0000) > knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00005644d0556ca8 CR3: 0000000002214006 CR4: 00000000003606f0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > gf100_gr_init_ctxctl+0x27b/0x2d0 [nouveau] > gf100_gr_init+0x5bd/0x5e0 [nouveau] > gf100_gr_init_+0x61/0x70 [nouveau] > nvkm_gr_init+0x1d/0x20 [nouveau] > nvkm_engine_init+0xcb/0x210 [nouveau] > nvkm_subdev_init+0xd6/0x230 [nouveau] > nvkm_engine_ref.part.0+0x52/0x70 [nouveau] > nvkm_engine_ref+0x13/0x20 [nouveau] > nvkm_ioctl_new+0x12c/0x260 [nouveau] > ? nvkm_fifo_chan_child_del+0xa0/0xa0 [nouveau] > ? gf100_gr_dtor+0xe0/0xe0 [nouveau] > nvkm_ioctl+0xe2/0x180 [nouveau] > nvkm_client_ioctl+0x12/0x20 [nouveau] > nvif_object_ioctl+0x47/0x50 [nouveau] > nvif_object_init+0xc8/0x120 [nouveau] > nvc0_fbcon_accel_init+0x5c/0x960 [nouveau] > nouveau_fbcon_create+0x5a5/0x5d0 [nouveau] > ? drm_setup_crtcs+0x27b/0xcb0 [drm_kms_helper] > ? __lock_is_held+0x5e/0xa0 > __drm_fb_helper_initial_config_and_unlock+0x27c/0x520 [drm_kms_helper] > drm_fb_helper_hotplug_event.part.29+0xae/0xc0 [drm_kms_helper] > drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper] > nouveau_fbcon_output_poll_changed+0xb8/0x110 [nouveau] > drm_kms_helper_hotplug_event+0x2a/0x40 [drm_kms_helper] > drm_dp_send_link_address+0x176/0x1c0 [drm_kms_helper] > drm_dp_check_and_send_link_address+0xa0/0xb0 [drm_kms_helper] > drm_dp_mst_link_probe_work+0xa4/0xc0 [drm_kms_helper] > process_one_work+0x22f/0x5c0 > worker_thread+0x44/0x3a0 > kthread+0x12b/0x150 > ? wq_pool_ids_show+0x140/0x140 > ? kthread_create_on_node+0x60/0x60 > ret_from_fork+0x3a/0x50 > irq event stamp: 22490 > hardirqs last enabled at (22489): [<ffffffff8113281d>] > console_unlock+0x44d/0x5f0 > hardirqs last disabled at (22490): [<ffffffff81001c03>] > trace_hardirqs_off_thunk+0x1a/0x1c > softirqs last enabled at (22486): [<ffffffff81c00330>] > __do_softirq+0x330/0x44d > softirqs last disabled at (22479): [<ffffffff810c3105>] > irq_exit+0xe5/0xf0 > WARNING: CPU: 0 PID: 12 at > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > gf100_grctx_generate+0x7b2/0x850 [nouveau] > ---[ end trace bf0976ed88b122a8 ]--- > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) > nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 engine > 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 > unknown] > > From which the GPU never manages to recover. Booting without nouveau > loading causes issues as well, since the GPU starts sending spurious > interrupts that cause other device's IRQs to get disabled by the kernel: > > irq 16: nobody cared (try booting with the "irqpoll" option) > … > handlers: > [<000000007faa9e99>] i801_isr [i2c_i801] > Disabling IRQ #16 > … > serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > i801_smbus 0000:00:1f.4: Transaction timeout > rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX > register (-110). > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > i801_smbus 0000:00:1f.4: Transaction timeout > rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change enabled > interrupts! > > Which in turn causes the touchpad and sometimes even other things to get > disabled. > > Since the GPU staying on causes problems even without nouveau's > intervention, we can't fix this problem from nouveau itself. We have to > fix it as early as possible in the boot sequence in order to make sure > that the GPU is in a clean state before it has a chance to spam us with > interrupts and break things. Was nouveau loaded *before* the reboot? Or can you reproduce the spurious interrupts even if you do a cold reboot without nouveau, then a warm reboot again without nouveau? > So to do this, we add a new pci quirk using > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe > at boot finishes. From there, we check to make sure that this is indeed > the specific P50 variant of this GPU. We also make sure that the GPU PCI > device is advertising NoReset- in order to prevent us from trying to > reset the GPU when the machine is in Dedicated graphics mode (where the > GPU being initialized by the BIOS is normal and expected). Finally, we > try mapping the MMIO space for the GPU which should only work if the GPU > is actually active in D0 mode. We can then read the magic 0x2240c > register on the GPU, which will have bit 1 set if the GPU's firmware has > already been posted during a previous boot. Once we've confirmed all of > this, we reset the PCI device and re-disable it - bringing the GPU back > into a healthy state. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: nouveau@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: Karol Herbst <kherbst@redhat.com> > Cc: Ben Skeggs <skeggsb@gmail.com> > Cc: stable@vger.kernel.org > --- > drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index b0a413f3f7ca..948492fda8bf 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > + > +/* > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a Nvidia > + * Quadro M1000M, the BIOS will occasionally make the mistake of not resetting > + * the nvidia GPU between reboots if the system is configured to use hybrid > + * graphics mode. This results in the GPU being left in whatever state it was > + * in during the previous boot which causes spurious interrupts from the GPU, > + * which in turn cause us to disable the wrong IRQs and end up breaking the > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > + * > + * Luckily, it seems a simple reset of the PCI device for the nvidia GPU > + * manages to bring the GPU back into a clean state and fix all of these > + * issues. Additionally since the GPU will report NoReset+ when the machine is > + * configured in Dedicated display mode, we don't need to worry about > + * accidentally resetting the GPU when it's supposed to already be > + * initialized. > + */ > +static void > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > +{ > + void __iomem *map; > + int ret; > + > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > + pdev->subsystem_device != 0x222e || > + !pdev->reset_fn) > + return; > + > + /* > + * If we can't enable the device's mmio space, it's probably not even > + * initialized. This is fine, and means we can just skip the quirk > + * entirely. > + */ > + if (pci_enable_device_mem(pdev)) { > + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); > + return; > + } > + > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > + if (!map) { > + pci_err(pdev, "Can't map MMIO space, this is probably very bad\n"); > + goto out_disable; > + } > + > + /* > + * Be extra careful, and make sure that the GPU firmware is posted > + * before trying a reset > + */ > + if (ioread32(map + 0x2240c) & 0x2) { > + pci_info(pdev, > + FW_BUG "GPU left initialized by EFI, resetting\n"); > + ret = pci_reset_function(pdev); > + if (ret < 0) > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > + } > + > + iounmap(map); > +out_disable: > + pci_disable_device(pdev); > +} > + > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > + PCI_CLASS_DISPLAY_VGA, 8, > + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot); > -- > 2.20.1 >
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.20.8, v4.19.21, v4.14.99, v4.9.156, v4.4.174, v3.18.134.
v4.20.8: Build OK!
v4.19.21: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
v4.14.99: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
06dc4ee54e30 ("PCI: Disable MSI for Freescale Layerscape PCIe RC mode")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
v4.9.156: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
3e13676862f9 ("thunderbolt: Add support for DMA configuration based mailbox")
46cd4b75cd0e ("efi: Add device path parser")
58c5475aba67 ("x86/efi: Retrieve and assign Apple device properties")
630b3aff8a51 ("treewide: Consolidate Apple DMI checks")
81a54b5e1986 ("thunderbolt: Let the connection manager handle all notifications")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
9d3cce0b6136 ("thunderbolt: Introduce thunderbolt bus and connection manager")
ac6c44de503e ("thunderbolt: Expose get_route() to other files")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
bfe778ac4982 ("thunderbolt: Convert switch to a device")
c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI")
da2da04b8d44 ("thunderbolt: Rework capability handling")
f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v4.4.174: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
14d2000182ed ("drm/radeon: Defer probe if gmux is present but its driver isn't")
156d7d4120e1 ("vga_switcheroo: Add handler flags infrastructure")
3a848662c751 ("vga_switcheroo: Prettify documentation")
412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded")
704ab614ec12 ("drm/i915: Defer probe if gmux is present but its driver isn't")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
989561de9b51 ("PM / Domains: add setter for dev.pm_domain")
98b3a3402eb6 ("drm/nouveau: Defer probe if gmux is present but its driver isn't")
a345918d6ee6 ("vga_switcheroo: Support deferred probing of audio clients")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
b00e5334ab1b ("vga_switcheroo: Add helper for deferred probing")
b5f88dd1d6ef ("Revert "ACPI / LPSS: allow to use specific PM domain during ->probe()"")
c1e1655bb892 ("apple-gmux: Assign apple_gmux_data before registering")
c68f4528a2e9 ("drm/amdkfd: Track when module's init is complete")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v3.18.134: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
083dba02947d ("drm/nouveau/device: recognise GM204")
2f4a58e852d1 ("drm/nouveau/subdev: always upcast through nouveau_subdev()/nouveau_engine()")
4766ec53945f ("drm/nouveau/bios: add parsing of BIT M(v2) +0x03 table")
50e216d6e7c3 ("drm/nouveau/bios: add parsing of pmu image tables")
5444204036b2 ("drm/nouveau: switch to new-style timer macros")
7bb6d4428d3d ("drm/nouveau: move the (far too many...) different s/r paths to the same place")
8d5e3af15c79 ("drm/nouveau/device: Add support for GK208B, resolves bug 86935")
8d85e06b5e04 ("drm/nouveau/bios: add pci data structure parsing")
989aa5b76ad2 ("drm/nouveau/nvif: namespace of nvkm accessors (no binary change)")
a01ca78c8f11 ("drm/nouveau/nvif: simplify and tidy library interfaces")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
ad4a36263535 ("drm/nouveau/bios: split out shadow methods")
b71a1344ec20 ("drm/nouveau/bios: add NPDE parsing")
ba6e34e61271 ("drm/gm204/devinit: initial implementation")
be83cd4ef9a2 ("drm/nouveau: finalise nvkm namespace switch (no binary change)")
c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no code changes)")
cb8bb9cedb60 ("drm/nouveau/tmr: cosmetic changes")
d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
f3867f439fd6 ("drm/nouveau/clk: rename from clock (no binary change)")
f8a8546194d7 ("drm/nouveau/clk: allow non-blocking for nouveau_clock_astate()")
How should we proceed with this patch?
--
Thanks,
Sasha
On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > Hi Lyude, > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > On a very specific subset of ThinkPad P50 SKUs, particularly ones that > > come with a Quadro M1000M chip instead of the M2000M variant, the BIOS > > seems to have a very nasty habit of not always resetting the secondary > > Nvidia GPU between full reboots if the laptop is configured in Hybrid > > Graphics mode. The reason for this happening is unknown, but the > > following steps and possibly a good bit of patience will reproduce the > > issue: > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > 2. Make sure nouveau is loaded and that the GPU is awake > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > > 4. If nouveau loads up properly, reboot the machine again and go back to > > step 2 until you reproduce the issue > > > > This results in some very strange behavior: the GPU will > > quite literally be left in exactly the same state it was in when the > > previously booted kernel started the reboot. This has all sorts of bad > > sideaffects: for starters, this completely breaks nouveau starting with a > > mysterious EVO channel failure that happens well before we've actually > > used the EVO channel for anything: > > There are a lot of moving parts here that are probably obvious to you > but not to me. I need help untangling this a bit so I'm comfortable > that we got to the root cause and that we're doing something logical > as opposed to something that just happens to make things work. I > really don't know enough to even ask the right questions... I completely understand! I'm pretty sure I'd be just as skeptical if I was in your position reviewing a patch like this :P > > Is there a bug report for this? Bugzilla.kernel.org would be ideal, > including "lspci -vvxxx" and dmidecode for the system. > Not yet, but there has been discussion about this between nouveau developers on our IRC channel. > Is this running a current BIOS? The date in your log below looks > pretty recent, so I assume it is current. Yes, this is the most up to date BIOS available for this system. > > I assume "hybrid graphics" means you have two GPUs. Do you select > hybrid graphics mode in the BIOS? Yes, the P50 has two available modes in the BIOS: Dedicated (e.g. only the nvidia GPU is used for everything), and Hybrid (i915 drives the built-in display panel, nouveau drives everything else). This bug only seems to occur in Hybrid mode. > > I assume when you say the Nvidia GPU doesn't get reset on a full > reboot, you're talking about a "warm reboot", and that if you actually > remove the power and do a cold reboot, there's no problem? If you meant "unplugging the power adapter" when you said cutting the power we don't need to go that far, but shutting down the machine and restarting it by hand does avoid the problem yes. > > I assume Nvidia GPU being active means you are using the performance > GPU. Does that mean the integrated GPU is completely unused and Linux > does nothing at all with it? Is Linux doing any switching between > them? If so, how? I am not 100% confident in the code I've seen that > does the switching. "Switching" isn't really the right word to describe it these days as the process for how this is handled has changed quite a bit in the last few years. As I mentioned above, the main intel GPU is used for driving the built-in display. As for the nvidia GPU, it's used to drive any kind of external displays connected to the P50 and can also be used to do rendering through DRI PRIME. The connector hookups are mostly hardcoded as there isn't really much of a mux, unlike the old days where we could actually do things like switch which GPU was driving the internal display on the fly with vga-switcheroo using the _DSM methods provided by ACPI. Nowadays muxless laptops like the ThinkPad P50 don't make much use of _DSM for power management and instead just runtime suspend the dedicated nvidia GPU when it's not in use. We rely on the PCI subsystem to invoke the _PR3 ACPI methods for suspending and resuming the GPU and don't handle much of it ourselves anymore: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nouveau_drm.c?h=v5.0-rc6#n860 (You can ignore the call to nouveau_switcheroo_optimus_dsm() there; it doesn't do anything on systems like the P50 where we detect _PR3) Additionally-if you see any problems with how we're handling things there we're all ears! We've been trying to fix as many issues with runtime PM as we can find. Something else to consider here too: I've only ever managed to reproduce this issue on this specific P50 SKU. Since I work on Desktop engineering at Red Hat we have access to a very large number of laptops with hybrid GPU setups. Interestingly I haven't ever reproduced anything like this on any of them, even P50 SKUs with the M2000M GPU instead of the M1000M (which are almost identical, they're both GM107 chips) I can follow the same steps that I followed to reproduce this bug and it never appears. Another thing to note: this bug also doesn't appear to happen 100% of the time. Some warm reboots the GPU will be reset by the BIOS perfectly fine. > > > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > > 00000002 > > > > Later on, this causes us to timeout trying to bring up the GR ctx: > > > > ------------[ cut here ]------------ > > nouveau 0000:01:00.0: timeout > > WARNING: CPU: 0 PID: 12 at > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > Modules linked in: nouveau mxm_wmi i915 crc32c_intel ttm i2c_algo_bit > > serio_raw drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops > > xhci_pci drm xhci_hcd i2c_core wmi video > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc5Lyude-Test+ #29 > > Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) > > 12/18/2018 > > Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] > > RIP: 0010:gf100_grctx_generate+0x7b2/0x850 [nouveau] > > Code: 85 d2 75 04 48 8b 57 10 48 89 95 28 ff ff ff e8 b4 37 0e e1 48 8b > > 95 28 ff ff ff 48 c7 c7 b1 97 57 a0 48 89 c6 e8 5a 38 c0 e0 <0f> 0b e9 > > b9 fd ff ff 48 8b 85 60 ff ff ff 48 8b 40 10 48 8b 78 10 > > RSP: 0018:ffffc900000b77f0 EFLAGS: 00010286 > > RAX: 0000000000000000 RBX: ffff888871af8000 RCX: 0000000000000000 > > RDX: ffff88887f41dfe0 RSI: ffff88887f415698 RDI: ffff88887f415698 > > RBP: ffffc900000b78c8 R08: 0000000000000000 R09: 0000000000000000 > > R10: 0000000000000000 R11: 0000000000000000 R12: ffff888872118000 > > R13: 0000000000000000 R14: ffffffffa0551420 R15: ffffc900000b7818 > > FS: 0000000000000000(0000) GS:ffff88887f400000(0000) > > knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00005644d0556ca8 CR3: 0000000002214006 CR4: 00000000003606f0 > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > Call Trace: > > gf100_gr_init_ctxctl+0x27b/0x2d0 [nouveau] > > gf100_gr_init+0x5bd/0x5e0 [nouveau] > > gf100_gr_init_+0x61/0x70 [nouveau] > > nvkm_gr_init+0x1d/0x20 [nouveau] > > nvkm_engine_init+0xcb/0x210 [nouveau] > > nvkm_subdev_init+0xd6/0x230 [nouveau] > > nvkm_engine_ref.part.0+0x52/0x70 [nouveau] > > nvkm_engine_ref+0x13/0x20 [nouveau] > > nvkm_ioctl_new+0x12c/0x260 [nouveau] > > ? nvkm_fifo_chan_child_del+0xa0/0xa0 [nouveau] > > ? gf100_gr_dtor+0xe0/0xe0 [nouveau] > > nvkm_ioctl+0xe2/0x180 [nouveau] > > nvkm_client_ioctl+0x12/0x20 [nouveau] > > nvif_object_ioctl+0x47/0x50 [nouveau] > > nvif_object_init+0xc8/0x120 [nouveau] > > nvc0_fbcon_accel_init+0x5c/0x960 [nouveau] > > nouveau_fbcon_create+0x5a5/0x5d0 [nouveau] > > ? drm_setup_crtcs+0x27b/0xcb0 [drm_kms_helper] > > ? __lock_is_held+0x5e/0xa0 > > __drm_fb_helper_initial_config_and_unlock+0x27c/0x520 [drm_kms_helper] > > drm_fb_helper_hotplug_event.part.29+0xae/0xc0 [drm_kms_helper] > > drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper] > > nouveau_fbcon_output_poll_changed+0xb8/0x110 [nouveau] > > drm_kms_helper_hotplug_event+0x2a/0x40 [drm_kms_helper] > > drm_dp_send_link_address+0x176/0x1c0 [drm_kms_helper] > > drm_dp_check_and_send_link_address+0xa0/0xb0 [drm_kms_helper] > > drm_dp_mst_link_probe_work+0xa4/0xc0 [drm_kms_helper] > > process_one_work+0x22f/0x5c0 > > worker_thread+0x44/0x3a0 > > kthread+0x12b/0x150 > > ? wq_pool_ids_show+0x140/0x140 > > ? kthread_create_on_node+0x60/0x60 > > ret_from_fork+0x3a/0x50 > > irq event stamp: 22490 > > hardirqs last enabled at (22489): [<ffffffff8113281d>] > > console_unlock+0x44d/0x5f0 > > hardirqs last disabled at (22490): [<ffffffff81001c03>] > > trace_hardirqs_off_thunk+0x1a/0x1c > > softirqs last enabled at (22486): [<ffffffff81c00330>] > > __do_softirq+0x330/0x44d > > softirqs last disabled at (22479): [<ffffffff810c3105>] > > irq_exit+0xe5/0xf0 > > WARNING: CPU: 0 PID: 12 at > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > ---[ end trace bf0976ed88b122a8 ]--- > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) > > nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 engine > > 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 > > unknown] > > > > From which the GPU never manages to recover. Booting without nouveau > > loading causes issues as well, since the GPU starts sending spurious > > interrupts that cause other device's IRQs to get disabled by the kernel: > > > > irq 16: nobody cared (try booting with the "irqpoll" option) > > … > > handlers: > > [<000000007faa9e99>] i801_isr [i2c_i801] > > Disabling IRQ #16 > > … > > serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > i801_smbus 0000:00:1f.4: Transaction timeout > > rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX > > register (-110). > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > i801_smbus 0000:00:1f.4: Transaction timeout > > rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change enabled > > interrupts! > > > > Which in turn causes the touchpad and sometimes even other things to get > > disabled. > > > > Since the GPU staying on causes problems even without nouveau's > > intervention, we can't fix this problem from nouveau itself. We have to > > fix it as early as possible in the boot sequence in order to make sure > > that the GPU is in a clean state before it has a chance to spam us with > > interrupts and break things. > > Was nouveau loaded *before* the reboot? Or can you reproduce the > spurious interrupts even if you do a cold reboot without nouveau, then > a warm reboot again without nouveau? Nouveau was loaded before the reboot, yes, and as far as I'm aware this bug won't show up following the steps you described here. We wouldn't really notice the interrupts if this bug happened without nouveau loaded (since the BIOS doesn't arm them). However, I can confirm it's being reset each on each warm reboot when nouveau's completely disabled by sticking values into some MMIO registers we know the VBIOS doesn't touch followed by rebooting in order to see if they retain their value (this is how I discovered that the GPU wasn't being power cycled in the first place, if the values I stick into said registers gets cleared after reboot then the GPU was reset, otherwise it wasn't). > > > So to do this, we add a new pci quirk using > > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe > > at boot finishes. From there, we check to make sure that this is indeed > > the specific P50 variant of this GPU. We also make sure that the GPU PCI > > device is advertising NoReset- in order to prevent us from trying to > > reset the GPU when the machine is in Dedicated graphics mode (where the > > GPU being initialized by the BIOS is normal and expected). Finally, we > > try mapping the MMIO space for the GPU which should only work if the GPU > > is actually active in D0 mode. We can then read the magic 0x2240c > > register on the GPU, which will have bit 1 set if the GPU's firmware has > > already been posted during a previous boot. Once we've confirmed all of > > this, we reset the PCI device and re-disable it - bringing the GPU back > > into a healthy state. > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > Cc: nouveau@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: Karol Herbst <kherbst@redhat.com> > > Cc: Ben Skeggs <skeggsb@gmail.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 65 insertions(+) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index b0a413f3f7ca..948492fda8bf 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > > + > > +/* > > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a Nvidia > > + * Quadro M1000M, the BIOS will occasionally make the mistake of not > > resetting > > + * the nvidia GPU between reboots if the system is configured to use > > hybrid > > + * graphics mode. This results in the GPU being left in whatever state it > > was > > + * in during the previous boot which causes spurious interrupts from the > > GPU, > > + * which in turn cause us to disable the wrong IRQs and end up breaking > > the > > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > > + * > > + * Luckily, it seems a simple reset of the PCI device for the nvidia GPU > > + * manages to bring the GPU back into a clean state and fix all of these > > + * issues. Additionally since the GPU will report NoReset+ when the > > machine is > > + * configured in Dedicated display mode, we don't need to worry about > > + * accidentally resetting the GPU when it's supposed to already be > > + * initialized. > > + */ > > +static void > > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > > +{ > > + void __iomem *map; > > + int ret; > > + > > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > > + pdev->subsystem_device != 0x222e || > > + !pdev->reset_fn) > > + return; > > + > > + /* > > + * If we can't enable the device's mmio space, it's probably not even > > + * initialized. This is fine, and means we can just skip the quirk > > + * entirely. > > + */ > > + if (pci_enable_device_mem(pdev)) { > > + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); > > + return; > > + } > > + > > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > > + if (!map) { > > + pci_err(pdev, "Can't map MMIO space, this is probably very > > bad\n"); > > + goto out_disable; > > + } > > + > > + /* > > + * Be extra careful, and make sure that the GPU firmware is posted > > + * before trying a reset > > + */ > > + if (ioread32(map + 0x2240c) & 0x2) { > > + pci_info(pdev, > > + FW_BUG "GPU left initialized by EFI, resetting\n"); > > + ret = pci_reset_function(pdev); > > + if (ret < 0) > > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > > + } > > + > > + iounmap(map); > > +out_disable: > > + pci_disable_device(pdev); > > +} > > + > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > + PCI_CLASS_DISPLAY_VGA, 8, > > + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot) > > ; > > -- > > 2.20.1 > >
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.20.8, v4.19.21, v4.14.99, v4.9.156, v4.4.174, v3.18.134.
v4.20.8: Build OK!
v4.19.21: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
v4.14.99: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
06dc4ee54e30 ("PCI: Disable MSI for Freescale Layerscape PCIe RC mode")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
v4.9.156: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
3e13676862f9 ("thunderbolt: Add support for DMA configuration based mailbox")
46cd4b75cd0e ("efi: Add device path parser")
58c5475aba67 ("x86/efi: Retrieve and assign Apple device properties")
630b3aff8a51 ("treewide: Consolidate Apple DMI checks")
81a54b5e1986 ("thunderbolt: Let the connection manager handle all notifications")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
9d3cce0b6136 ("thunderbolt: Introduce thunderbolt bus and connection manager")
ac6c44de503e ("thunderbolt: Expose get_route() to other files")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
bfe778ac4982 ("thunderbolt: Convert switch to a device")
c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI")
da2da04b8d44 ("thunderbolt: Rework capability handling")
f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v4.4.174: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
14d2000182ed ("drm/radeon: Defer probe if gmux is present but its driver isn't")
156d7d4120e1 ("vga_switcheroo: Add handler flags infrastructure")
3a848662c751 ("vga_switcheroo: Prettify documentation")
412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded")
704ab614ec12 ("drm/i915: Defer probe if gmux is present but its driver isn't")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
989561de9b51 ("PM / Domains: add setter for dev.pm_domain")
98b3a3402eb6 ("drm/nouveau: Defer probe if gmux is present but its driver isn't")
a345918d6ee6 ("vga_switcheroo: Support deferred probing of audio clients")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
b00e5334ab1b ("vga_switcheroo: Add helper for deferred probing")
b5f88dd1d6ef ("Revert "ACPI / LPSS: allow to use specific PM domain during ->probe()"")
c1e1655bb892 ("apple-gmux: Assign apple_gmux_data before registering")
c68f4528a2e9 ("drm/amdkfd: Track when module's init is complete")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v3.18.134: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
083dba02947d ("drm/nouveau/device: recognise GM204")
2f4a58e852d1 ("drm/nouveau/subdev: always upcast through nouveau_subdev()/nouveau_engine()")
4766ec53945f ("drm/nouveau/bios: add parsing of BIT M(v2) +0x03 table")
50e216d6e7c3 ("drm/nouveau/bios: add parsing of pmu image tables")
5444204036b2 ("drm/nouveau: switch to new-style timer macros")
7bb6d4428d3d ("drm/nouveau: move the (far too many...) different s/r paths to the same place")
8d5e3af15c79 ("drm/nouveau/device: Add support for GK208B, resolves bug 86935")
8d85e06b5e04 ("drm/nouveau/bios: add pci data structure parsing")
989aa5b76ad2 ("drm/nouveau/nvif: namespace of nvkm accessors (no binary change)")
a01ca78c8f11 ("drm/nouveau/nvif: simplify and tidy library interfaces")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
ad4a36263535 ("drm/nouveau/bios: split out shadow methods")
b71a1344ec20 ("drm/nouveau/bios: add NPDE parsing")
ba6e34e61271 ("drm/gm204/devinit: initial implementation")
be83cd4ef9a2 ("drm/nouveau: finalise nvkm namespace switch (no binary change)")
c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no code changes)")
cb8bb9cedb60 ("drm/nouveau/tmr: cosmetic changes")
d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
f3867f439fd6 ("drm/nouveau/clk: rename from clock (no binary change)")
f8a8546194d7 ("drm/nouveau/clk: allow non-blocking for nouveau_clock_astate()")
How should we proceed with this patch?
--
Thanks,
Sasha
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.20.8, v4.19.21, v4.14.99, v4.9.156, v4.4.174, v3.18.134.
v4.20.8: Build OK!
v4.19.21: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
v4.14.99: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
06dc4ee54e30 ("PCI: Disable MSI for Freescale Layerscape PCIe RC mode")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
v4.9.156: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
3e13676862f9 ("thunderbolt: Add support for DMA configuration based mailbox")
46cd4b75cd0e ("efi: Add device path parser")
58c5475aba67 ("x86/efi: Retrieve and assign Apple device properties")
630b3aff8a51 ("treewide: Consolidate Apple DMI checks")
81a54b5e1986 ("thunderbolt: Let the connection manager handle all notifications")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
9d3cce0b6136 ("thunderbolt: Introduce thunderbolt bus and connection manager")
ac6c44de503e ("thunderbolt: Expose get_route() to other files")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
bfe778ac4982 ("thunderbolt: Convert switch to a device")
c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI")
da2da04b8d44 ("thunderbolt: Rework capability handling")
f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v4.4.174: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
14d2000182ed ("drm/radeon: Defer probe if gmux is present but its driver isn't")
156d7d4120e1 ("vga_switcheroo: Add handler flags infrastructure")
3a848662c751 ("vga_switcheroo: Prettify documentation")
412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded")
704ab614ec12 ("drm/i915: Defer probe if gmux is present but its driver isn't")
8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking")
989561de9b51 ("PM / Domains: add setter for dev.pm_domain")
98b3a3402eb6 ("drm/nouveau: Defer probe if gmux is present but its driver isn't")
a345918d6ee6 ("vga_switcheroo: Support deferred probing of audio clients")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
b00e5334ab1b ("vga_switcheroo: Add helper for deferred probing")
b5f88dd1d6ef ("Revert "ACPI / LPSS: allow to use specific PM domain during ->probe()"")
c1e1655bb892 ("apple-gmux: Assign apple_gmux_data before registering")
c68f4528a2e9 ("drm/amdkfd: Track when module's init is complete")
fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues")
v3.18.134: Failed to apply! Possible dependencies:
01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations")
07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller")
083dba02947d ("drm/nouveau/device: recognise GM204")
2f4a58e852d1 ("drm/nouveau/subdev: always upcast through nouveau_subdev()/nouveau_engine()")
4766ec53945f ("drm/nouveau/bios: add parsing of BIT M(v2) +0x03 table")
50e216d6e7c3 ("drm/nouveau/bios: add parsing of pmu image tables")
5444204036b2 ("drm/nouveau: switch to new-style timer macros")
7bb6d4428d3d ("drm/nouveau: move the (far too many...) different s/r paths to the same place")
8d5e3af15c79 ("drm/nouveau/device: Add support for GK208B, resolves bug 86935")
8d85e06b5e04 ("drm/nouveau/bios: add pci data structure parsing")
989aa5b76ad2 ("drm/nouveau/nvif: namespace of nvkm accessors (no binary change)")
a01ca78c8f11 ("drm/nouveau/nvif: simplify and tidy library interfaces")
ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB")
ad4a36263535 ("drm/nouveau/bios: split out shadow methods")
b71a1344ec20 ("drm/nouveau/bios: add NPDE parsing")
ba6e34e61271 ("drm/gm204/devinit: initial implementation")
be83cd4ef9a2 ("drm/nouveau: finalise nvkm namespace switch (no binary change)")
c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no code changes)")
cb8bb9cedb60 ("drm/nouveau/tmr: cosmetic changes")
d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
f3867f439fd6 ("drm/nouveau/clk: rename from clock (no binary change)")
f8a8546194d7 ("drm/nouveau/clk: allow non-blocking for nouveau_clock_astate()")
How should we proceed with this patch?
--
Thanks,
Sasha
On Mon, Feb 18, 2019 at 3:14 PM Sasha Levin <sashal@kernel.org> wrote: > > Hi, > > [This is an automated email] > > This commit has been processed because it contains a -stable tag. > The stable tag indicates that it's relevant for the following trees: all > > The bot has tested the following trees: v4.20.8, v4.19.21, v4.14.99, v4.9.156, v4.4.174, v3.18.134. > > v4.20.8: Build OK! > v4.19.21: Failed to apply! Possible dependencies: > 01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations") > > v4.14.99: Failed to apply! Possible dependencies: > 01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations") > 06dc4ee54e30 ("PCI: Disable MSI for Freescale Layerscape PCIe RC mode") > 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") > 8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking") > ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB") > > v4.9.156: Failed to apply! Possible dependencies: > 01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations") > 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") > 3e13676862f9 ("thunderbolt: Add support for DMA configuration based mailbox") > 46cd4b75cd0e ("efi: Add device path parser") > 58c5475aba67 ("x86/efi: Retrieve and assign Apple device properties") > 630b3aff8a51 ("treewide: Consolidate Apple DMI checks") > 81a54b5e1986 ("thunderbolt: Let the connection manager handle all notifications") > 8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking") > 9d3cce0b6136 ("thunderbolt: Introduce thunderbolt bus and connection manager") > ac6c44de503e ("thunderbolt: Expose get_route() to other files") > ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB") > bfe778ac4982 ("thunderbolt: Convert switch to a device") > c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI") > da2da04b8d44 ("thunderbolt: Rework capability handling") > f67cf491175a ("thunderbolt: Add support for Internal Connection Manager (ICM)") > fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues") > > v4.4.174: Failed to apply! Possible dependencies: > 01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations") > 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") > 14d2000182ed ("drm/radeon: Defer probe if gmux is present but its driver isn't") > 156d7d4120e1 ("vga_switcheroo: Add handler flags infrastructure") > 3a848662c751 ("vga_switcheroo: Prettify documentation") > 412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when amdkfd not loaded") > 704ab614ec12 ("drm/i915: Defer probe if gmux is present but its driver isn't") > 8948ca1a12c9 ("vga_switcheroo: Deduplicate power state tracking") > 989561de9b51 ("PM / Domains: add setter for dev.pm_domain") > 98b3a3402eb6 ("drm/nouveau: Defer probe if gmux is present but its driver isn't") > a345918d6ee6 ("vga_switcheroo: Support deferred probing of audio clients") > ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB") > b00e5334ab1b ("vga_switcheroo: Add helper for deferred probing") > b5f88dd1d6ef ("Revert "ACPI / LPSS: allow to use specific PM domain during ->probe()"") > c1e1655bb892 ("apple-gmux: Assign apple_gmux_data before registering") > c68f4528a2e9 ("drm/amdkfd: Track when module's init is complete") > fa6d513aefe4 ("drivers:gpu: vga :vga_switcheroo.c : Fixed some coding style issues") > > v3.18.134: Failed to apply! Possible dependencies: > 01d5d7fa8376 ("PCI: Add macro for Switchtec quirk declarations") > 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") > 083dba02947d ("drm/nouveau/device: recognise GM204") > 2f4a58e852d1 ("drm/nouveau/subdev: always upcast through nouveau_subdev()/nouveau_engine()") > 4766ec53945f ("drm/nouveau/bios: add parsing of BIT M(v2) +0x03 table") > 50e216d6e7c3 ("drm/nouveau/bios: add parsing of pmu image tables") > 5444204036b2 ("drm/nouveau: switch to new-style timer macros") > 7bb6d4428d3d ("drm/nouveau: move the (far too many...) different s/r paths to the same place") > 8d5e3af15c79 ("drm/nouveau/device: Add support for GK208B, resolves bug 86935") > 8d85e06b5e04 ("drm/nouveau/bios: add pci data structure parsing") > 989aa5b76ad2 ("drm/nouveau/nvif: namespace of nvkm accessors (no binary change)") > a01ca78c8f11 ("drm/nouveau/nvif: simplify and tidy library interfaces") > ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi Switchtec NTB") > ad4a36263535 ("drm/nouveau/bios: split out shadow methods") > b71a1344ec20 ("drm/nouveau/bios: add NPDE parsing") > ba6e34e61271 ("drm/gm204/devinit: initial implementation") > be83cd4ef9a2 ("drm/nouveau: finalise nvkm namespace switch (no binary change)") > c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no code changes)") > cb8bb9cedb60 ("drm/nouveau/tmr: cosmetic changes") > d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)") > f3867f439fd6 ("drm/nouveau/clk: rename from clock (no binary change)") > f8a8546194d7 ("drm/nouveau/clk: allow non-blocking for nouveau_clock_astate()") > > > How should we proceed with this patch? Is there any way we can turn off this automated checking for a while? At this stage, where we're just trying to discuss the issue, these emails just clutter things up.
[note to David Ober: you -should- be able to reply to this, hopefully, but I haven't actually tested that so results may vary] Hi again! Sorry I didn't fully answer all of the questions you originally asked in this email, I had to get in contact with Lenovo to make sure that it was OK for me to disclose more details on this bug (and I had PTO scheduled immediately after I asked). I've added David Ober from Lenovo to this thread as well. So now that I've got Lenovo's approval I can answer those questions, and give some better answers for the others! (see below) On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > Hi Lyude, > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > On a very specific subset of ThinkPad P50 SKUs, particularly ones that > > > come with a Quadro M1000M chip instead of the M2000M variant, the BIOS > > > seems to have a very nasty habit of not always resetting the secondary > > > Nvidia GPU between full reboots if the laptop is configured in Hybrid > > > Graphics mode. The reason for this happening is unknown, but the > > > following steps and possibly a good bit of patience will reproduce the > > > issue: > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > > > 4. If nouveau loads up properly, reboot the machine again and go back to > > > step 2 until you reproduce the issue > > > > > > This results in some very strange behavior: the GPU will > > > quite literally be left in exactly the same state it was in when the > > > previously booted kernel started the reboot. This has all sorts of bad > > > sideaffects: for starters, this completely breaks nouveau starting with > > > a > > > mysterious EVO channel failure that happens well before we've actually > > > used the EVO channel for anything: > > > > There are a lot of moving parts here that are probably obvious to you > > but not to me. I need help untangling this a bit so I'm comfortable > > that we got to the root cause and that we're doing something logical > > as opposed to something that just happens to make things work. I > > really don't know enough to even ask the right questions... > > I completely understand! I'm pretty sure I'd be just as skeptical if I was > in > your position reviewing a patch like this :P > > > Is there a bug report for this? Bugzilla.kernel.org would be ideal, > > including "lspci -vvxxx" and dmidecode for the system. > > > Not yet, but there has been discussion about this between nouveau developers > on our IRC channel. I lied: yes there actually is a bug report for this, but it's currently on the Red Hat bugzilla. I can get more information from it if you need (with lenovo's approval of course). > > > Is this running a current BIOS? The date in your log below looks > > pretty recent, so I assume it is current. > > Yes, this is the most up to date BIOS available for this system. And additionally: I've been working with Lenovo on this issue for a couple of months now, and we've gone through dozens of different trial BIOSes with no success thus far. However, Lenovo is currently working on trying to add this workaround into their BIOS but I've been told that this change is going to take a decent amount of time since they need to test it across multiple operating systems. I'd be happy to come back and add a conditional later to turn this workaround off for later BIOS versions once Lenovo has released a proper fix. With all of that being said, [how] do you think we should proceed? > > > I assume "hybrid graphics" means you have two GPUs. Do you select > > hybrid graphics mode in the BIOS? > > Yes, the P50 has two available modes in the BIOS: Dedicated (e.g. only > the nvidia GPU is used for everything), and Hybrid (i915 drives the > built-in display panel, nouveau drives everything else). This bug only > seems to occur in Hybrid mode. > > > I assume when you say the Nvidia GPU doesn't get reset on a full > > reboot, you're talking about a "warm reboot", and that if you actually > > remove the power and do a cold reboot, there's no problem? > > If you meant "unplugging the power adapter" when you said cutting the > power we don't need to go that far, but shutting down the machine and > restarting it by hand does avoid the problem yes. > > > I assume Nvidia GPU being active means you are using the performance > > GPU. Does that mean the integrated GPU is completely unused and Linux > > does nothing at all with it? Is Linux doing any switching between > > them? If so, how? I am not 100% confident in the code I've seen that > > does the switching. > > "Switching" isn't really the right word to describe it these days as the > process for how this is handled has changed quite a bit in the last few > years. As I mentioned above, the main intel GPU is used for driving the > built-in display. As for the nvidia GPU, it's used to drive any kind of > external displays connected to the P50 and can also be used to do > rendering through DRI PRIME. The connector hookups are mostly hardcoded > as there isn't really much of a mux, unlike the old days where we could > actually do things like switch which GPU was driving the internal > display on the fly with vga-switcheroo using the _DSM methods provided > by ACPI. > > Nowadays muxless laptops like the ThinkPad P50 don't make much use of > _DSM for power management and instead just runtime suspend the dedicated > nvidia GPU when it's not in use. We rely on the PCI subsystem to invoke > the _PR3 ACPI methods for suspending and resuming the GPU and don't > handle much of it ourselves anymore: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nouveau_drm.c?h=v5.0-rc6#n860 > > (You can ignore the call to nouveau_switcheroo_optimus_dsm() there; it > doesn't do anything on systems like the P50 where we detect _PR3) > > Additionally-if you see any problems with how we're handling things > there we're all ears! We've been trying to fix as many issues with > runtime PM as we can find. > > Something else to consider here too: I've only ever managed to reproduce > this issue on this specific P50 SKU. Since I work on Desktop engineering > at Red Hat we have access to a very large number of laptops with hybrid > GPU setups. Interestingly I haven't ever reproduced anything like this > on any of them, even P50 SKUs with the M2000M GPU instead of the M1000M > (which are almost identical, they're both GM107 chips) I can follow the > same steps that I followed to reproduce this bug and it never appears. > > Another thing to note: this bug also doesn't appear to happen 100% of > the time. Some warm reboots the GPU will be reset by the BIOS perfectly > fine. > > > > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > > > 00000002 > > > > > > Later on, this causes us to timeout trying to bring up the GR ctx: > > > > > > ------------[ cut here ]------------ > > > nouveau 0000:01:00.0: timeout > > > WARNING: CPU: 0 PID: 12 at > > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > Modules linked in: nouveau mxm_wmi i915 crc32c_intel ttm i2c_algo_bit > > > serio_raw drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops > > > xhci_pci drm xhci_hcd i2c_core wmi video > > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc5Lyude-Test+ #29 > > > Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) > > > 12/18/2018 > > > Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] > > > RIP: 0010:gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > Code: 85 d2 75 04 48 8b 57 10 48 89 95 28 ff ff ff e8 b4 37 0e e1 48 8b > > > 95 28 ff ff ff 48 c7 c7 b1 97 57 a0 48 89 c6 e8 5a 38 c0 e0 <0f> 0b e9 > > > b9 fd ff ff 48 8b 85 60 ff ff ff 48 8b 40 10 48 8b 78 10 > > > RSP: 0018:ffffc900000b77f0 EFLAGS: 00010286 > > > RAX: 0000000000000000 RBX: ffff888871af8000 RCX: 0000000000000000 > > > RDX: ffff88887f41dfe0 RSI: ffff88887f415698 RDI: ffff88887f415698 > > > RBP: ffffc900000b78c8 R08: 0000000000000000 R09: 0000000000000000 > > > R10: 0000000000000000 R11: 0000000000000000 R12: ffff888872118000 > > > R13: 0000000000000000 R14: ffffffffa0551420 R15: ffffc900000b7818 > > > FS: 0000000000000000(0000) GS:ffff88887f400000(0000) > > > knlGS:0000000000000000 > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > CR2: 00005644d0556ca8 CR3: 0000000002214006 CR4: 00000000003606f0 > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > Call Trace: > > > gf100_gr_init_ctxctl+0x27b/0x2d0 [nouveau] > > > gf100_gr_init+0x5bd/0x5e0 [nouveau] > > > gf100_gr_init_+0x61/0x70 [nouveau] > > > nvkm_gr_init+0x1d/0x20 [nouveau] > > > nvkm_engine_init+0xcb/0x210 [nouveau] > > > nvkm_subdev_init+0xd6/0x230 [nouveau] > > > nvkm_engine_ref.part.0+0x52/0x70 [nouveau] > > > nvkm_engine_ref+0x13/0x20 [nouveau] > > > nvkm_ioctl_new+0x12c/0x260 [nouveau] > > > ? nvkm_fifo_chan_child_del+0xa0/0xa0 [nouveau] > > > ? gf100_gr_dtor+0xe0/0xe0 [nouveau] > > > nvkm_ioctl+0xe2/0x180 [nouveau] > > > nvkm_client_ioctl+0x12/0x20 [nouveau] > > > nvif_object_ioctl+0x47/0x50 [nouveau] > > > nvif_object_init+0xc8/0x120 [nouveau] > > > nvc0_fbcon_accel_init+0x5c/0x960 [nouveau] > > > nouveau_fbcon_create+0x5a5/0x5d0 [nouveau] > > > ? drm_setup_crtcs+0x27b/0xcb0 [drm_kms_helper] > > > ? __lock_is_held+0x5e/0xa0 > > > __drm_fb_helper_initial_config_and_unlock+0x27c/0x520 [drm_kms_helper] > > > drm_fb_helper_hotplug_event.part.29+0xae/0xc0 [drm_kms_helper] > > > drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper] > > > nouveau_fbcon_output_poll_changed+0xb8/0x110 [nouveau] > > > drm_kms_helper_hotplug_event+0x2a/0x40 [drm_kms_helper] > > > drm_dp_send_link_address+0x176/0x1c0 [drm_kms_helper] > > > drm_dp_check_and_send_link_address+0xa0/0xb0 [drm_kms_helper] > > > drm_dp_mst_link_probe_work+0xa4/0xc0 [drm_kms_helper] > > > process_one_work+0x22f/0x5c0 > > > worker_thread+0x44/0x3a0 > > > kthread+0x12b/0x150 > > > ? wq_pool_ids_show+0x140/0x140 > > > ? kthread_create_on_node+0x60/0x60 > > > ret_from_fork+0x3a/0x50 > > > irq event stamp: 22490 > > > hardirqs last enabled at (22489): [<ffffffff8113281d>] > > > console_unlock+0x44d/0x5f0 > > > hardirqs last disabled at (22490): [<ffffffff81001c03>] > > > trace_hardirqs_off_thunk+0x1a/0x1c > > > softirqs last enabled at (22486): [<ffffffff81c00330>] > > > __do_softirq+0x330/0x44d > > > softirqs last disabled at (22479): [<ffffffff810c3105>] > > > irq_exit+0xe5/0xf0 > > > WARNING: CPU: 0 PID: 12 at > > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > ---[ end trace bf0976ed88b122a8 ]--- > > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: > > > 1) > > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: > > > 1) > > > nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 engine > > > 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 > > > unknown] > > > > > > From which the GPU never manages to recover. Booting without nouveau > > > loading causes issues as well, since the GPU starts sending spurious > > > interrupts that cause other device's IRQs to get disabled by the kernel: > > > > > > irq 16: nobody cared (try booting with the "irqpoll" option) > > > … > > > handlers: > > > [<000000007faa9e99>] i801_isr [i2c_i801] > > > Disabling IRQ #16 > > > … > > > serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 > > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > > i801_smbus 0000:00:1f.4: Transaction timeout > > > rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX > > > register (-110). > > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > > i801_smbus 0000:00:1f.4: Transaction timeout > > > rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change enabled > > > interrupts! > > > > > > Which in turn causes the touchpad and sometimes even other things to get > > > disabled. > > > > > > Since the GPU staying on causes problems even without nouveau's > > > intervention, we can't fix this problem from nouveau itself. We have to > > > fix it as early as possible in the boot sequence in order to make sure > > > that the GPU is in a clean state before it has a chance to spam us with > > > interrupts and break things. > > > > Was nouveau loaded *before* the reboot? Or can you reproduce the > > spurious interrupts even if you do a cold reboot without nouveau, then > > a warm reboot again without nouveau? > > Nouveau was loaded before the reboot, yes, and as far as I'm aware this > bug won't show up following the steps you described here. We wouldn't > really notice the interrupts if this bug happened without nouveau loaded > (since the BIOS doesn't arm them). However, I can confirm it's being > reset each on each warm reboot when nouveau's completely disabled by > sticking values into some MMIO registers we know the VBIOS doesn't touch > followed by rebooting in order to see if they retain their value (this > is how I discovered that the GPU wasn't being power cycled in the first > place, if the values I stick into said registers gets cleared after > reboot then the GPU was reset, otherwise it wasn't). > > > > > So to do this, we add a new pci quirk using > > > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe > > > at boot finishes. From there, we check to make sure that this is indeed > > > the specific P50 variant of this GPU. We also make sure that the GPU PCI > > > device is advertising NoReset- in order to prevent us from trying to > > > reset the GPU when the machine is in Dedicated graphics mode (where the > > > GPU being initialized by the BIOS is normal and expected). Finally, we > > > try mapping the MMIO space for the GPU which should only work if the GPU > > > is actually active in D0 mode. We can then read the magic 0x2240c > > > register on the GPU, which will have bit 1 set if the GPU's firmware has > > > already been posted during a previous boot. Once we've confirmed all of > > > this, we reset the PCI device and re-disable it - bringing the GPU back > > > into a healthy state. > > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > Cc: nouveau@lists.freedesktop.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: Karol Herbst <kherbst@redhat.com> > > > Cc: Ben Skeggs <skeggsb@gmail.com> > > > Cc: stable@vger.kernel.org > > > --- > > > drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 65 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index b0a413f3f7ca..948492fda8bf 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > > > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > > > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > > > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > > > + > > > +/* > > > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a > > > Nvidia > > > + * Quadro M1000M, the BIOS will occasionally make the mistake of not > > > resetting > > > + * the nvidia GPU between reboots if the system is configured to use > > > hybrid > > > + * graphics mode. This results in the GPU being left in whatever state > > > it > > > was > > > + * in during the previous boot which causes spurious interrupts from > > > the > > > GPU, > > > + * which in turn cause us to disable the wrong IRQs and end up breaking > > > the > > > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > > > + * > > > + * Luckily, it seems a simple reset of the PCI device for the nvidia > > > GPU > > > + * manages to bring the GPU back into a clean state and fix all of > > > these > > > + * issues. Additionally since the GPU will report NoReset+ when the > > > machine is > > > + * configured in Dedicated display mode, we don't need to worry about > > > + * accidentally resetting the GPU when it's supposed to already be > > > + * initialized. > > > + */ > > > +static void > > > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > > > +{ > > > + void __iomem *map; > > > + int ret; > > > + > > > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > > > + pdev->subsystem_device != 0x222e || > > > + !pdev->reset_fn) > > > + return; > > > + > > > + /* > > > + * If we can't enable the device's mmio space, it's probably not even > > > + * initialized. This is fine, and means we can just skip the quirk > > > + * entirely. > > > + */ > > > + if (pci_enable_device_mem(pdev)) { > > > + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); > > > + return; > > > + } > > > + > > > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > > > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > > > + if (!map) { > > > + pci_err(pdev, "Can't map MMIO space, this is probably very > > > bad\n"); > > > + goto out_disable; > > > + } > > > + > > > + /* > > > + * Be extra careful, and make sure that the GPU firmware is posted > > > + * before trying a reset > > > + */ > > > + if (ioread32(map + 0x2240c) & 0x2) { > > > + pci_info(pdev, > > > + FW_BUG "GPU left initialized by EFI, resetting\n"); > > > + ret = pci_reset_function(pdev); > > > + if (ret < 0) > > > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > > > + } > > > + > > > + iounmap(map); > > > +out_disable: > > > + pci_disable_device(pdev); > > > +} > > > + > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > > + PCI_CLASS_DISPLAY_VGA, 8, > > > + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot) > > > ; > > > -- > > > 2.20.1 > > >
Bump, sorry to bug you but is there any update on this or any information you still need from me which would help get this upstream? On Wed, 2019-03-13 at 18:25 -0400, Lyude Paul wrote: > [note to David Ober: you -should- be able to reply to this, hopefully, but I > haven't actually tested that so results may vary] > > Hi again! Sorry I didn't fully answer all of the questions you originally > asked in this email, I had to get in contact with Lenovo to make sure that > it > was OK for me to disclose more details on this bug (and I had PTO scheduled > immediately after I asked). I've added David Ober from Lenovo to this thread > as well. So now that I've got Lenovo's approval I can answer those > questions, > and give some better answers for the others! (see below) > > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > Hi Lyude, > > > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > On a very specific subset of ThinkPad P50 SKUs, particularly ones that > > > > come with a Quadro M1000M chip instead of the M2000M variant, the BIOS > > > > seems to have a very nasty habit of not always resetting the secondary > > > > Nvidia GPU between full reboots if the laptop is configured in Hybrid > > > > Graphics mode. The reason for this happening is unknown, but the > > > > following steps and possibly a good bit of patience will reproduce the > > > > issue: > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may > > > > help) > > > > 4. If nouveau loads up properly, reboot the machine again and go back > > > > to > > > > step 2 until you reproduce the issue > > > > > > > > This results in some very strange behavior: the GPU will > > > > quite literally be left in exactly the same state it was in when the > > > > previously booted kernel started the reboot. This has all sorts of bad > > > > sideaffects: for starters, this completely breaks nouveau starting > > > > with > > > > a > > > > mysterious EVO channel failure that happens well before we've actually > > > > used the EVO channel for anything: > > > > > > There are a lot of moving parts here that are probably obvious to you > > > but not to me. I need help untangling this a bit so I'm comfortable > > > that we got to the root cause and that we're doing something logical > > > as opposed to something that just happens to make things work. I > > > really don't know enough to even ask the right questions... > > > > I completely understand! I'm pretty sure I'd be just as skeptical if I was > > in > > your position reviewing a patch like this :P > > > > > Is there a bug report for this? Bugzilla.kernel.org would be ideal, > > > including "lspci -vvxxx" and dmidecode for the system. > > > > > Not yet, but there has been discussion about this between nouveau > > developers > > on our IRC channel. > I lied: yes there actually is a bug report for this, but it's currently on > the > Red Hat bugzilla. I can get more information from it if you need (with > lenovo's approval of course). > > > > Is this running a current BIOS? The date in your log below looks > > > pretty recent, so I assume it is current. > > > > Yes, this is the most up to date BIOS available for this system. > > And additionally: I've been working with Lenovo on this issue for a couple > of > months now, and we've gone through dozens of different trial BIOSes with no > success thus far. However, Lenovo is currently working on trying to add this > workaround into their BIOS but I've been told that this change is going to > take a decent amount of time since they need to test it across multiple > operating systems. I'd be happy to come back and add a conditional later to > turn this workaround off for later BIOS versions once Lenovo has released a > proper fix. > > With all of that being said, [how] do you think we should proceed? > > > > I assume "hybrid graphics" means you have two GPUs. Do you select > > > hybrid graphics mode in the BIOS? > > > > Yes, the P50 has two available modes in the BIOS: Dedicated (e.g. only > > the nvidia GPU is used for everything), and Hybrid (i915 drives the > > built-in display panel, nouveau drives everything else). This bug only > > seems to occur in Hybrid mode. > > > > > I assume when you say the Nvidia GPU doesn't get reset on a full > > > reboot, you're talking about a "warm reboot", and that if you actually > > > remove the power and do a cold reboot, there's no problem? > > > > If you meant "unplugging the power adapter" when you said cutting the > > power we don't need to go that far, but shutting down the machine and > > restarting it by hand does avoid the problem yes. > > > > > I assume Nvidia GPU being active means you are using the performance > > > GPU. Does that mean the integrated GPU is completely unused and Linux > > > does nothing at all with it? Is Linux doing any switching between > > > them? If so, how? I am not 100% confident in the code I've seen that > > > does the switching. > > > > "Switching" isn't really the right word to describe it these days as the > > process for how this is handled has changed quite a bit in the last few > > years. As I mentioned above, the main intel GPU is used for driving the > > built-in display. As for the nvidia GPU, it's used to drive any kind of > > external displays connected to the P50 and can also be used to do > > rendering through DRI PRIME. The connector hookups are mostly hardcoded > > as there isn't really much of a mux, unlike the old days where we could > > actually do things like switch which GPU was driving the internal > > display on the fly with vga-switcheroo using the _DSM methods provided > > by ACPI. > > > > Nowadays muxless laptops like the ThinkPad P50 don't make much use of > > _DSM for power management and instead just runtime suspend the dedicated > > nvidia GPU when it's not in use. We rely on the PCI subsystem to invoke > > the _PR3 ACPI methods for suspending and resuming the GPU and don't > > handle much of it ourselves anymore: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nouveau_drm.c?h=v5.0-rc6#n860 > > > > (You can ignore the call to nouveau_switcheroo_optimus_dsm() there; it > > doesn't do anything on systems like the P50 where we detect _PR3) > > > > Additionally-if you see any problems with how we're handling things > > there we're all ears! We've been trying to fix as many issues with > > runtime PM as we can find. > > > > Something else to consider here too: I've only ever managed to reproduce > > this issue on this specific P50 SKU. Since I work on Desktop engineering > > at Red Hat we have access to a very large number of laptops with hybrid > > GPU setups. Interestingly I haven't ever reproduced anything like this > > on any of them, even P50 SKUs with the M2000M GPU instead of the M1000M > > (which are almost identical, they're both GM107 chips) I can follow the > > same steps that I followed to reproduce this bug and it never appears. > > > > Another thing to note: this bug also doesn't appear to happen 100% of > > the time. Some warm reboots the GPU will be reset by the BIOS perfectly > > fine. > > > > > > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > > > > 00000002 > > > > > > > > Later on, this causes us to timeout trying to bring up the GR ctx: > > > > > > > > ------------[ cut here ]------------ > > > > nouveau 0000:01:00.0: timeout > > > > WARNING: CPU: 0 PID: 12 at > > > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > > Modules linked in: nouveau mxm_wmi i915 crc32c_intel ttm i2c_algo_bit > > > > serio_raw drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops > > > > xhci_pci drm xhci_hcd i2c_core wmi video > > > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc5Lyude-Test+ #29 > > > > Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) > > > > 12/18/2018 > > > > Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] > > > > RIP: 0010:gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > > Code: 85 d2 75 04 48 8b 57 10 48 89 95 28 ff ff ff e8 b4 37 0e e1 48 > > > > 8b > > > > 95 28 ff ff ff 48 c7 c7 b1 97 57 a0 48 89 c6 e8 5a 38 c0 e0 <0f> 0b e9 > > > > b9 fd ff ff 48 8b 85 60 ff ff ff 48 8b 40 10 48 8b 78 10 > > > > RSP: 0018:ffffc900000b77f0 EFLAGS: 00010286 > > > > RAX: 0000000000000000 RBX: ffff888871af8000 RCX: 0000000000000000 > > > > RDX: ffff88887f41dfe0 RSI: ffff88887f415698 RDI: ffff88887f415698 > > > > RBP: ffffc900000b78c8 R08: 0000000000000000 R09: 0000000000000000 > > > > R10: 0000000000000000 R11: 0000000000000000 R12: ffff888872118000 > > > > R13: 0000000000000000 R14: ffffffffa0551420 R15: ffffc900000b7818 > > > > FS: 0000000000000000(0000) GS:ffff88887f400000(0000) > > > > knlGS:0000000000000000 > > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > CR2: 00005644d0556ca8 CR3: 0000000002214006 CR4: 00000000003606f0 > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > > Call Trace: > > > > gf100_gr_init_ctxctl+0x27b/0x2d0 [nouveau] > > > > gf100_gr_init+0x5bd/0x5e0 [nouveau] > > > > gf100_gr_init_+0x61/0x70 [nouveau] > > > > nvkm_gr_init+0x1d/0x20 [nouveau] > > > > nvkm_engine_init+0xcb/0x210 [nouveau] > > > > nvkm_subdev_init+0xd6/0x230 [nouveau] > > > > nvkm_engine_ref.part.0+0x52/0x70 [nouveau] > > > > nvkm_engine_ref+0x13/0x20 [nouveau] > > > > nvkm_ioctl_new+0x12c/0x260 [nouveau] > > > > ? nvkm_fifo_chan_child_del+0xa0/0xa0 [nouveau] > > > > ? gf100_gr_dtor+0xe0/0xe0 [nouveau] > > > > nvkm_ioctl+0xe2/0x180 [nouveau] > > > > nvkm_client_ioctl+0x12/0x20 [nouveau] > > > > nvif_object_ioctl+0x47/0x50 [nouveau] > > > > nvif_object_init+0xc8/0x120 [nouveau] > > > > nvc0_fbcon_accel_init+0x5c/0x960 [nouveau] > > > > nouveau_fbcon_create+0x5a5/0x5d0 [nouveau] > > > > ? drm_setup_crtcs+0x27b/0xcb0 [drm_kms_helper] > > > > ? __lock_is_held+0x5e/0xa0 > > > > __drm_fb_helper_initial_config_and_unlock+0x27c/0x520 > > > > [drm_kms_helper] > > > > drm_fb_helper_hotplug_event.part.29+0xae/0xc0 [drm_kms_helper] > > > > drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper] > > > > nouveau_fbcon_output_poll_changed+0xb8/0x110 [nouveau] > > > > drm_kms_helper_hotplug_event+0x2a/0x40 [drm_kms_helper] > > > > drm_dp_send_link_address+0x176/0x1c0 [drm_kms_helper] > > > > drm_dp_check_and_send_link_address+0xa0/0xb0 [drm_kms_helper] > > > > drm_dp_mst_link_probe_work+0xa4/0xc0 [drm_kms_helper] > > > > process_one_work+0x22f/0x5c0 > > > > worker_thread+0x44/0x3a0 > > > > kthread+0x12b/0x150 > > > > ? wq_pool_ids_show+0x140/0x140 > > > > ? kthread_create_on_node+0x60/0x60 > > > > ret_from_fork+0x3a/0x50 > > > > irq event stamp: 22490 > > > > hardirqs last enabled at (22489): [<ffffffff8113281d>] > > > > console_unlock+0x44d/0x5f0 > > > > hardirqs last disabled at (22490): [<ffffffff81001c03>] > > > > trace_hardirqs_off_thunk+0x1a/0x1c > > > > softirqs last enabled at (22486): [<ffffffff81c00330>] > > > > __do_softirq+0x330/0x44d > > > > softirqs last disabled at (22479): [<ffffffff810c3105>] > > > > irq_exit+0xe5/0xf0 > > > > WARNING: CPU: 0 PID: 12 at > > > > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > > > > gf100_grctx_generate+0x7b2/0x850 [nouveau] > > > > ---[ end trace bf0976ed88b122a8 ]--- > > > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, > > > > busy: > > > > 1) > > > > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, > > > > busy: > > > > 1) > > > > nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 > > > > engine > > > > 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 > > > > unknown] > > > > > > > > From which the GPU never manages to recover. Booting without nouveau > > > > loading causes issues as well, since the GPU starts sending spurious > > > > interrupts that cause other device's IRQs to get disabled by the > > > > kernel: > > > > > > > > irq 16: nobody cared (try booting with the "irqpoll" option) > > > > … > > > > handlers: > > > > [<000000007faa9e99>] i801_isr [i2c_i801] > > > > Disabling IRQ #16 > > > > … > > > > serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 > > > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > > > i801_smbus 0000:00:1f.4: Transaction timeout > > > > rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX > > > > register (-110). > > > > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > > > > i801_smbus 0000:00:1f.4: Transaction timeout > > > > rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change > > > > enabled > > > > interrupts! > > > > > > > > Which in turn causes the touchpad and sometimes even other things to > > > > get > > > > disabled. > > > > > > > > Since the GPU staying on causes problems even without nouveau's > > > > intervention, we can't fix this problem from nouveau itself. We have > > > > to > > > > fix it as early as possible in the boot sequence in order to make sure > > > > that the GPU is in a clean state before it has a chance to spam us > > > > with > > > > interrupts and break things. > > > > > > Was nouveau loaded *before* the reboot? Or can you reproduce the > > > spurious interrupts even if you do a cold reboot without nouveau, then > > > a warm reboot again without nouveau? > > > > Nouveau was loaded before the reboot, yes, and as far as I'm aware this > > bug won't show up following the steps you described here. We wouldn't > > really notice the interrupts if this bug happened without nouveau loaded > > (since the BIOS doesn't arm them). However, I can confirm it's being > > reset each on each warm reboot when nouveau's completely disabled by > > sticking values into some MMIO registers we know the VBIOS doesn't touch > > followed by rebooting in order to see if they retain their value (this > > is how I discovered that the GPU wasn't being power cycled in the first > > place, if the values I stick into said registers gets cleared after > > reboot then the GPU was reset, otherwise it wasn't). > > > > > > > > So to do this, we add a new pci quirk using > > > > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI > > > > probe > > > > at boot finishes. From there, we check to make sure that this is > > > > indeed > > > > the specific P50 variant of this GPU. We also make sure that the GPU > > > > PCI > > > > device is advertising NoReset- in order to prevent us from trying to > > > > reset the GPU when the machine is in Dedicated graphics mode (where > > > > the > > > > GPU being initialized by the BIOS is normal and expected). Finally, we > > > > try mapping the MMIO space for the GPU which should only work if the > > > > GPU > > > > is actually active in D0 mode. We can then read the magic 0x2240c > > > > register on the GPU, which will have bit 1 set if the GPU's firmware > > > > has > > > > already been posted during a previous boot. Once we've confirmed all > > > > of > > > > this, we reset the PCI device and re-disable it - bringing the GPU > > > > back > > > > into a healthy state. > > > > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > > Cc: nouveau@lists.freedesktop.org > > > > Cc: dri-devel@lists.freedesktop.org > > > > Cc: Karol Herbst <kherbst@redhat.com> > > > > Cc: Ben Skeggs <skeggsb@gmail.com> > > > > Cc: stable@vger.kernel.org > > > > --- > > > > drivers/pci/quirks.c | 65 > > > > ++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 65 insertions(+) > > > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > > index b0a413f3f7ca..948492fda8bf 100644 > > > > --- a/drivers/pci/quirks.c > > > > +++ b/drivers/pci/quirks.c > > > > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > > > > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > > > > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > > > > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > > > > + > > > > +/* > > > > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a > > > > Nvidia > > > > + * Quadro M1000M, the BIOS will occasionally make the mistake of not > > > > resetting > > > > + * the nvidia GPU between reboots if the system is configured to use > > > > hybrid > > > > + * graphics mode. This results in the GPU being left in whatever > > > > state > > > > it > > > > was > > > > + * in during the previous boot which causes spurious interrupts from > > > > the > > > > GPU, > > > > + * which in turn cause us to disable the wrong IRQs and end up > > > > breaking > > > > the > > > > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > > > > + * > > > > + * Luckily, it seems a simple reset of the PCI device for the nvidia > > > > GPU > > > > + * manages to bring the GPU back into a clean state and fix all of > > > > these > > > > + * issues. Additionally since the GPU will report NoReset+ when the > > > > machine is > > > > + * configured in Dedicated display mode, we don't need to worry about > > > > + * accidentally resetting the GPU when it's supposed to already be > > > > + * initialized. > > > > + */ > > > > +static void > > > > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > > > > +{ > > > > + void __iomem *map; > > > > + int ret; > > > > + > > > > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > > > > + pdev->subsystem_device != 0x222e || > > > > + !pdev->reset_fn) > > > > + return; > > > > + > > > > + /* > > > > + * If we can't enable the device's mmio space, it's probably > > > > not even > > > > + * initialized. This is fine, and means we can just skip the > > > > quirk > > > > + * entirely. > > > > + */ > > > > + if (pci_enable_device_mem(pdev)) { > > > > + pci_dbg(pdev, "Can't enable device mem, no reset > > > > needed\n"); > > > > + return; > > > > + } > > > > + > > > > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > > > > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > > > > + if (!map) { > > > > + pci_err(pdev, "Can't map MMIO space, this is probably > > > > very > > > > bad\n"); > > > > + goto out_disable; > > > > + } > > > > + > > > > + /* > > > > + * Be extra careful, and make sure that the GPU firmware is > > > > posted > > > > + * before trying a reset > > > > + */ > > > > + if (ioread32(map + 0x2240c) & 0x2) { > > > > + pci_info(pdev, > > > > + FW_BUG "GPU left initialized by EFI, > > > > resetting\n"); > > > > + ret = pci_reset_function(pdev); > > > > + if (ret < 0) > > > > + pci_err(pdev, "Failed to reset GPU: %d\n", > > > > ret); > > > > + } > > > > + > > > > + iounmap(map); > > > > +out_disable: > > > > + pci_disable_device(pdev); > > > > +} > > > > + > > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > > > + PCI_CLASS_DISPLAY_VGA, 8, > > > > + quirk_lenovo_thinkpad_p50_nvgpu_survives > > > > _reboot) > > > > ; > > > > -- > > > > 2.20.1 > > > >
[+cc Rafael] On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > variant, the BIOS seems to have a very nasty habit of not > > > > always resetting the secondary Nvidia GPU between full reboots > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > reason for this happening is unknown, but the following steps > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > > > > 4. If nouveau loads up properly, reboot the machine again and go back to > > > > step 2 until you reproduce the issue > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > literally be left in exactly the same state it was in when the > > > > previously booted kernel started the reboot. This has all > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > nouveau starting with a mysterious EVO channel failure that > > > > happens well before we've actually used the EVO channel for > > > > anything: Thanks for the hybrid tutorial (snipped from this response). IIUC, what you said was that in hybrid mode, the Intel GPU drives the built-in display and the Nvidia GPU drives any external displays and may be used for DRI PRIME rendering (whatever that is). But since you say the Nvidia device gets runtime suspended, I assume there's no external display here and you're not using DRI PRIME. I wonder if it's related to the fact that the Nvidia GPU has been runtime suspended before you do the reboot. Can you try turning of runtime power management for the GPU by setting the runpm module parameter to 0? I *think* this would be booting with "nouveau.runpm=0". > > > Is there a bug report for this? Bugzilla.kernel.org would be ideal, > > > including "lspci -vvxxx" and dmidecode for the system. > > > > > Not yet, but there has been discussion about this between nouveau > > developers on our IRC channel. > > I lied: yes there actually is a bug report for this, but it's > currently on the Red Hat bugzilla. I can get more information from > it if you need (with lenovo's approval of course). Can you please make a bugzilla.kernel.org entry with as much information (dmesg, "lspci -vvxxx", dmidecode, etc) as you can get approval for? You can include the Red Hat bugzilla URL in the commit log, too, but that's not quite as good because we have no control over whether it's public. > And additionally: I've been working with Lenovo on this issue for a > couple of months now, and we've gone through dozens of different > trial BIOSes with no success thus far. However, Lenovo is currently > working on trying to add this workaround into their BIOS but I've > been told that this change is going to take a decent amount of time > since they need to test it across multiple operating systems. I'd be > happy to come back and add a conditional later to turn this > workaround off for later BIOS versions once Lenovo has released a > proper fix. Sounds like Lenovo is going to a lot of trouble for this. The ideal thing from my point of view would be if they could figure out why this works on Windows but not on Linux. I doubt Windows has a quirk like this, so if we could figure out why it works on Windows, we could likely do something similar in Linux. > > > > So to do this, we add a new pci quirk using > > > > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe > > > > at boot finishes. From there, we check to make sure that this is indeed > > > > the specific P50 variant of this GPU. We also make sure that the GPU PCI > > > > device is advertising NoReset- in order to prevent us from trying to > > > > reset the GPU when the machine is in Dedicated graphics mode (where the > > > > GPU being initialized by the BIOS is normal and expected). Finally, we > > > > try mapping the MMIO space for the GPU which should only work if the GPU > > > > is actually active in D0 mode. We can then read the magic 0x2240c > > > > register on the GPU, which will have bit 1 set if the GPU's firmware has > > > > already been posted during a previous boot. Once we've confirmed all of > > > > this, we reset the PCI device and re-disable it - bringing the GPU back > > > > into a healthy state. > > > > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > > Cc: nouveau@lists.freedesktop.org > > > > Cc: dri-devel@lists.freedesktop.org > > > > Cc: Karol Herbst <kherbst@redhat.com> > > > > Cc: Ben Skeggs <skeggsb@gmail.com> > > > > Cc: stable@vger.kernel.org > > > > --- > > > > drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 65 insertions(+) > > > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > > index b0a413f3f7ca..948492fda8bf 100644 > > > > --- a/drivers/pci/quirks.c > > > > +++ b/drivers/pci/quirks.c > > > > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > > > > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > > > > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > > > > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > > > > + > > > > +/* > > > > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a > > > > Nvidia > > > > + * Quadro M1000M, the BIOS will occasionally make the mistake of not > > > > resetting > > > > + * the nvidia GPU between reboots if the system is configured to use > > > > hybrid > > > > + * graphics mode. This results in the GPU being left in whatever state > > > > it > > > > was > > > > + * in during the previous boot which causes spurious interrupts from > > > > the > > > > GPU, > > > > + * which in turn cause us to disable the wrong IRQs and end up breaking > > > > the > > > > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > > > > + * > > > > + * Luckily, it seems a simple reset of the PCI device for the nvidia > > > > GPU > > > > + * manages to bring the GPU back into a clean state and fix all of > > > > these > > > > + * issues. Additionally since the GPU will report NoReset+ when the > > > > machine is > > > > + * configured in Dedicated display mode, we don't need to worry about > > > > + * accidentally resetting the GPU when it's supposed to already be > > > > + * initialized. > > > > + */ > > > > +static void > > > > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > > > > +{ > > > > + void __iomem *map; > > > > + int ret; > > > > + > > > > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > > > > + pdev->subsystem_device != 0x222e || > > > > + !pdev->reset_fn) > > > > + return; > > > > + > > > > + /* > > > > + * If we can't enable the device's mmio space, it's probably not even > > > > + * initialized. This is fine, and means we can just skip the quirk > > > > + * entirely. > > > > + */ > > > > + if (pci_enable_device_mem(pdev)) { > > > > + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); > > > > + return; > > > > + } > > > > + > > > > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > > > > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > > > > + if (!map) { > > > > + pci_err(pdev, "Can't map MMIO space, this is probably very > > > > bad\n"); > > > > + goto out_disable; > > > > + } > > > > + > > > > + /* > > > > + * Be extra careful, and make sure that the GPU firmware is posted > > > > + * before trying a reset > > > > + */ > > > > + if (ioread32(map + 0x2240c) & 0x2) { > > > > + pci_info(pdev, > > > > + FW_BUG "GPU left initialized by EFI, resetting\n"); > > > > + ret = pci_reset_function(pdev); > > > > + if (ret < 0) > > > > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > > > > + } > > > > + > > > > + iounmap(map); > > > > +out_disable: > > > > + pci_disable_device(pdev); > > > > +} > > > > + > > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > > > + PCI_CLASS_DISPLAY_VGA, 8, > > > > + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot) > > > > ; > > > > -- > > > > 2.20.1 > > > > > -- > Cheers, > Lyude Paul >
On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote: > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > > variant, the BIOS seems to have a very nasty habit of not > > > > > always resetting the secondary Nvidia GPU between full reboots > > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > > reason for this happening is unknown, but the following steps > > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > > > > > 4. If nouveau loads up properly, reboot the machine again and go back to > > > > > step 2 until you reproduce the issue > > > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > > literally be left in exactly the same state it was in when the > > > > > previously booted kernel started the reboot. This has all > > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > > nouveau starting with a mysterious EVO channel failure that > > > > > happens well before we've actually used the EVO channel for > > > > > anything: > > Thanks for the hybrid tutorial (snipped from this response). IIUC, > what you said was that in hybrid mode, the Intel GPU drives the > built-in display and the Nvidia GPU drives any external displays and > may be used for DRI PRIME rendering (whatever that is). But since you > say the Nvidia device gets runtime suspended, I assume there's no > external display here and you're not using DRI PRIME. > > I wonder if it's related to the fact that the Nvidia GPU has been > runtime suspended before you do the reboot. Can you try turning of > runtime power management for the GPU by setting the runpm module > parameter to 0? I *think* this would be booting with > "nouveau.runpm=0". Sorry, I wasn't really thinking here. You already *said* this is related to runtime suspend. It only happens when the Nvidia GPU has been suspended. I don't know that much about suspend, but ISTR seeing comments about resuming devices before we shutdown. If we do that, maybe there's some kind of race between that resume and the reboot? Bjorn
Note: I did read your response lower down in the thread, but I wanted to make sure I addressed one of the comments here (see below) On Thu, 2019-03-21 at 17:48 -0500, Bjorn Helgaas wrote: > [+cc Rafael] > > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > > variant, the BIOS seems to have a very nasty habit of not > > > > > always resetting the secondary Nvidia GPU between full reboots > > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > > reason for this happening is unknown, but the following steps > > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may > > > > > help) > > > > > 4. If nouveau loads up properly, reboot the machine again and go > > > > > back to > > > > > step 2 until you reproduce the issue > > > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > > literally be left in exactly the same state it was in when the > > > > > previously booted kernel started the reboot. This has all > > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > > nouveau starting with a mysterious EVO channel failure that > > > > > happens well before we've actually used the EVO channel for > > > > > anything: > > Thanks for the hybrid tutorial (snipped from this response). IIUC, > what you said was that in hybrid mode, the Intel GPU drives the > built-in display and the Nvidia GPU drives any external displays and > may be used for DRI PRIME rendering (whatever that is). But since you > say the Nvidia device gets runtime suspended, I assume there's no > external display here and you're not using DRI PRIME. > > I wonder if it's related to the fact that the Nvidia GPU has been > runtime suspended before you do the reboot. Can you try turning of > runtime power management for the GPU by setting the runpm module > parameter to 0? I *think* this would be booting with > "nouveau.runpm=0". > > > > > Is there a bug report for this? Bugzilla.kernel.org would be ideal, > > > > including "lspci -vvxxx" and dmidecode for the system. > > > > > > > Not yet, but there has been discussion about this between nouveau > > > developers on our IRC channel. > > > > I lied: yes there actually is a bug report for this, but it's > > currently on the Red Hat bugzilla. I can get more information from > > it if you need (with lenovo's approval of course). > > Can you please make a bugzilla.kernel.org entry with as much > information (dmesg, "lspci -vvxxx", dmidecode, etc) as you can get > approval for? You can include the Red Hat bugzilla URL in the commit > log, too, but that's not quite as good because we have no control over > whether it's public. > > > And additionally: I've been working with Lenovo on this issue for a > > couple of months now, and we've gone through dozens of different > > trial BIOSes with no success thus far. However, Lenovo is currently > > working on trying to add this workaround into their BIOS but I've > > been told that this change is going to take a decent amount of time > > since they need to test it across multiple operating systems. I'd be > > happy to come back and add a conditional later to turn this > > workaround off for later BIOS versions once Lenovo has released a > > proper fix. > > Sounds like Lenovo is going to a lot of trouble for this. The ideal > thing from my point of view would be if they could figure out why this > works on Windows but not on Linux. I doubt Windows has a quirk like > this, so if we could figure out why it works on Windows, we could > likely do something similar in Linux. I did actually try this route after first finding this bug, but unfortunately from what I understand there isn't really much more Lenovo can do other then give us a patched BIOS or look at their own BIOS to see if it's the cause. Anyway, went ahead and filed a bug with as much information as I could get my hands on here (different email then the one I'm talking to you from): https://bugzilla.kernel.org/show_bug.cgi?id=203003 > > > > > > So to do this, we add a new pci quirk using > > > > > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI > > > > > probe > > > > > at boot finishes. From there, we check to make sure that this is > > > > > indeed > > > > > the specific P50 variant of this GPU. We also make sure that the GPU > > > > > PCI > > > > > device is advertising NoReset- in order to prevent us from trying to > > > > > reset the GPU when the machine is in Dedicated graphics mode (where > > > > > the > > > > > GPU being initialized by the BIOS is normal and expected). Finally, > > > > > we > > > > > try mapping the MMIO space for the GPU which should only work if the > > > > > GPU > > > > > is actually active in D0 mode. We can then read the magic 0x2240c > > > > > register on the GPU, which will have bit 1 set if the GPU's firmware > > > > > has > > > > > already been posted during a previous boot. Once we've confirmed all > > > > > of > > > > > this, we reset the PCI device and re-disable it - bringing the GPU > > > > > back > > > > > into a healthy state. > > > > > > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > > > Cc: nouveau@lists.freedesktop.org > > > > > Cc: dri-devel@lists.freedesktop.org > > > > > Cc: Karol Herbst <kherbst@redhat.com> > > > > > Cc: Ben Skeggs <skeggsb@gmail.com> > > > > > Cc: stable@vger.kernel.org > > > > > --- > > > > > drivers/pci/quirks.c | 65 > > > > > ++++++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 65 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > > > index b0a413f3f7ca..948492fda8bf 100644 > > > > > --- a/drivers/pci/quirks.c > > > > > +++ b/drivers/pci/quirks.c > > > > > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > > > > > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > > > > > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > > > > > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > > > > > + > > > > > +/* > > > > > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a > > > > > Nvidia > > > > > + * Quadro M1000M, the BIOS will occasionally make the mistake of > > > > > not > > > > > resetting > > > > > + * the nvidia GPU between reboots if the system is configured to > > > > > use > > > > > hybrid > > > > > + * graphics mode. This results in the GPU being left in whatever > > > > > state > > > > > it > > > > > was > > > > > + * in during the previous boot which causes spurious interrupts > > > > > from > > > > > the > > > > > GPU, > > > > > + * which in turn cause us to disable the wrong IRQs and end up > > > > > breaking > > > > > the > > > > > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > > > > > + * > > > > > + * Luckily, it seems a simple reset of the PCI device for the > > > > > nvidia > > > > > GPU > > > > > + * manages to bring the GPU back into a clean state and fix all of > > > > > these > > > > > + * issues. Additionally since the GPU will report NoReset+ when the > > > > > machine is > > > > > + * configured in Dedicated display mode, we don't need to worry > > > > > about > > > > > + * accidentally resetting the GPU when it's supposed to already be > > > > > + * initialized. > > > > > + */ > > > > > +static void > > > > > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev > > > > > *pdev) > > > > > +{ > > > > > + void __iomem *map; > > > > > + int ret; > > > > > + > > > > > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > > > > > + pdev->subsystem_device != 0x222e || > > > > > + !pdev->reset_fn) > > > > > + return; > > > > > + > > > > > + /* > > > > > + * If we can't enable the device's mmio space, it's probably > > > > > not even > > > > > + * initialized. This is fine, and means we can just skip the > > > > > quirk > > > > > + * entirely. > > > > > + */ > > > > > + if (pci_enable_device_mem(pdev)) { > > > > > + pci_dbg(pdev, "Can't enable device mem, no reset > > > > > needed\n"); > > > > > + return; > > > > > + } > > > > > + > > > > > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > > > > > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > > > > > + if (!map) { > > > > > + pci_err(pdev, "Can't map MMIO space, this is probably > > > > > very > > > > > bad\n"); > > > > > + goto out_disable; > > > > > + } > > > > > + > > > > > + /* > > > > > + * Be extra careful, and make sure that the GPU firmware is > > > > > posted > > > > > + * before trying a reset > > > > > + */ > > > > > + if (ioread32(map + 0x2240c) & 0x2) { > > > > > + pci_info(pdev, > > > > > + FW_BUG "GPU left initialized by EFI, > > > > > resetting\n"); > > > > > + ret = pci_reset_function(pdev); > > > > > + if (ret < 0) > > > > > + pci_err(pdev, "Failed to reset GPU: %d\n", > > > > > ret); > > > > > + } > > > > > + > > > > > + iounmap(map); > > > > > +out_disable: > > > > > + pci_disable_device(pdev); > > > > > +} > > > > > + > > > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > > > > > + PCI_CLASS_DISPLAY_VGA, 8, > > > > > + quirk_lenovo_thinkpad_p50_nvgpu_survives > > > > > _reboot) > > > > > ; > > > > > -- > > > > > 2.20.1 > > > > > > > -- > > Cheers, > > Lyude Paul > >
Hi, any update on this/do you guys need any more information here? Would very much like to get this upstream On Fri, 2019-03-22 at 06:30 -0500, Bjorn Helgaas wrote: > On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote: > > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > > > variant, the BIOS seems to have a very nasty habit of not > > > > > > always resetting the secondary Nvidia GPU between full reboots > > > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > > > reason for this happening is unknown, but the following steps > > > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle > > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may > > > > > > help) > > > > > > 4. If nouveau loads up properly, reboot the machine again and go > > > > > > back to > > > > > > step 2 until you reproduce the issue > > > > > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > > > literally be left in exactly the same state it was in when the > > > > > > previously booted kernel started the reboot. This has all > > > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > > > nouveau starting with a mysterious EVO channel failure that > > > > > > happens well before we've actually used the EVO channel for > > > > > > anything: > > > > Thanks for the hybrid tutorial (snipped from this response). IIUC, > > what you said was that in hybrid mode, the Intel GPU drives the > > built-in display and the Nvidia GPU drives any external displays and > > may be used for DRI PRIME rendering (whatever that is). But since you > > say the Nvidia device gets runtime suspended, I assume there's no > > external display here and you're not using DRI PRIME. > > > > I wonder if it's related to the fact that the Nvidia GPU has been > > runtime suspended before you do the reboot. Can you try turning of > > runtime power management for the GPU by setting the runpm module > > parameter to 0? I *think* this would be booting with > > "nouveau.runpm=0". > > Sorry, I wasn't really thinking here. You already *said* this is > related to runtime suspend. It only happens when the Nvidia GPU has > been suspended. > > I don't know that much about suspend, but ISTR seeing comments about > resuming devices before we shutdown. If we do that, maybe there's > some kind of race between that resume and the reboot? > > Bjorn
[+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) resume")]
On Fri, Mar 22, 2019 at 06:30:15AM -0500, Bjorn Helgaas wrote:
> On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote:
> > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote:
> > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote:
> > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote:
> > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote:
> > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly
> > > > > > ones that come with a Quadro M1000M chip instead of the M2000M
> > > > > > variant, the BIOS seems to have a very nasty habit of not
> > > > > > always resetting the secondary Nvidia GPU between full reboots
> > > > > > if the laptop is configured in Hybrid Graphics mode. The
> > > > > > reason for this happening is unknown, but the following steps
> > > > > > and possibly a good bit of patience will reproduce the issue:
> > > > > >
> > > > > > 1. Boot up the laptop normally in Hybrid graphics mode
> > > > > > 2. Make sure nouveau is loaded and that the GPU is awake
> > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being idle
> > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help)
> > > > > > 4. If nouveau loads up properly, reboot the machine again and go back to
> > > > > > step 2 until you reproduce the issue
> > > > > >
> > > > > > This results in some very strange behavior: the GPU will quite
> > > > > > literally be left in exactly the same state it was in when the
> > > > > > previously booted kernel started the reboot. This has all
> > > > > > sorts of bad sideaffects: for starters, this completely breaks
> > > > > > nouveau starting with a mysterious EVO channel failure that
> > > > > > happens well before we've actually used the EVO channel for
> > > > > > anything:
> >
> > Thanks for the hybrid tutorial (snipped from this response). IIUC,
> > what you said was that in hybrid mode, the Intel GPU drives the
> > built-in display and the Nvidia GPU drives any external displays and
> > may be used for DRI PRIME rendering (whatever that is). But since you
> > say the Nvidia device gets runtime suspended, I assume there's no
> > external display here and you're not using DRI PRIME.
> >
> > I wonder if it's related to the fact that the Nvidia GPU has been
> > runtime suspended before you do the reboot. Can you try turning of
> > runtime power management for the GPU by setting the runpm module
> > parameter to 0? I *think* this would be booting with
> > "nouveau.runpm=0".
>
> Sorry, I wasn't really thinking here. You already *said* this is
> related to runtime suspend. It only happens when the Nvidia GPU has
> been suspended.
>
> I don't know that much about suspend, but ISTR seeing comments about
> resuming devices before we shutdown. If we do that, maybe there's
> some kind of race between that resume and the reboot?
I think we do in fact resume PCI devices before shutdown. Here's the
path I'm looking at:
device_shutdown
pm_runtime_get_noresume
pm_runtime_barrier
dev->bus->shutdown
pci_device_shutdown
pm_runtime_resume
__pm_runtime_resume(dev, 0)
rpm_resume(dev, 0)
__update_runtime_status(dev, RPM_RESUMING)
callback = RPM_GET_CALLBACK(dev, runtime_resume)
rpm_callback(callback, dev)
__rpm_callback
pci_pm_runtime_resume
drv->pm->runtime_resume
nouveau_pmops_runtime_resume
nouveau_do_resume
schedule_work(hpd_work) # <---
...
nouveau_display_hpd_work
pm_runtime_get_sync
drm_helper_hpd_irq_event
pm_runtime_mark_last_busy
pm_runtime_put_sync
I'm curious about that "schedule_work(hpd_work)" near the end because
no other drivers seem to use schedule_work() in the runtime_resume
path, and I don't know how that synchronizes with the shutdown
process. I don't see anything that waits for
nouveau_display_hpd_work() to complete, so it seems like something
that could be a race.
I wonder this problem would be easier to reproduce if you added a
sleep in nouveau_display_hpd_work() as in the first hunk below, and I
wonder if the problem would then go away if you stopped scheduling
hpd_work as in the second hunk? Obviously the second hunk isn't a
solution, it's just an attempt to figure out if I'm looking in the
right area.
Bjorn
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 55c0fa451163..e50806012d41 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -350,6 +350,7 @@ nouveau_display_hpd_work(struct work_struct *work)
pm_runtime_get_sync(drm->dev->dev);
+ msleep(2000);
drm_helper_hpd_irq_event(drm->dev);
pm_runtime_mark_last_busy(drm->dev->dev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 5020265bfbd9..48da72caa017 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -946,9 +946,6 @@ nouveau_pmops_runtime_resume(struct device *dev)
nvif_mask(&device->object, 0x088488, (1 << 25), (1 << 25));
drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
- /* Monitors may have been connected / disconnected during suspend */
- schedule_work(&nouveau_drm(drm_dev)->hpd_work);
-
return ret;
}
On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote: > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > resume")] > > On Fri, Mar 22, 2019 at 06:30:15AM -0500, Bjorn Helgaas wrote: > > On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote: > > > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > > > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > > > > variant, the BIOS seems to have a very nasty habit of not > > > > > > > always resetting the secondary Nvidia GPU between full reboots > > > > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > > > > reason for this happening is unknown, but the following steps > > > > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being > > > > > > > idle > > > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b > > > > > > > may help) > > > > > > > 4. If nouveau loads up properly, reboot the machine again and go > > > > > > > back to > > > > > > > step 2 until you reproduce the issue > > > > > > > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > > > > literally be left in exactly the same state it was in when the > > > > > > > previously booted kernel started the reboot. This has all > > > > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > > > > nouveau starting with a mysterious EVO channel failure that > > > > > > > happens well before we've actually used the EVO channel for > > > > > > > anything: > > > > > > Thanks for the hybrid tutorial (snipped from this response). IIUC, > > > what you said was that in hybrid mode, the Intel GPU drives the > > > built-in display and the Nvidia GPU drives any external displays and > > > may be used for DRI PRIME rendering (whatever that is). But since you > > > say the Nvidia device gets runtime suspended, I assume there's no > > > external display here and you're not using DRI PRIME. > > > > > > I wonder if it's related to the fact that the Nvidia GPU has been > > > runtime suspended before you do the reboot. Can you try turning of > > > runtime power management for the GPU by setting the runpm module > > > parameter to 0? I *think* this would be booting with > > > "nouveau.runpm=0". > > > > Sorry, I wasn't really thinking here. You already *said* this is > > related to runtime suspend. It only happens when the Nvidia GPU has > > been suspended. > > > > I don't know that much about suspend, but ISTR seeing comments about > > resuming devices before we shutdown. If we do that, maybe there's > > some kind of race between that resume and the reboot? > > I think we do in fact resume PCI devices before shutdown. Here's the > path I'm looking at: > > device_shutdown > pm_runtime_get_noresume > pm_runtime_barrier > dev->bus->shutdown > pci_device_shutdown > pm_runtime_resume > __pm_runtime_resume(dev, 0) > rpm_resume(dev, 0) > __update_runtime_status(dev, RPM_RESUMING) > callback = RPM_GET_CALLBACK(dev, runtime_resume) > rpm_callback(callback, dev) > __rpm_callback > pci_pm_runtime_resume > drv->pm->runtime_resume > nouveau_pmops_runtime_resume > nouveau_do_resume > schedule_work(hpd_work) # <--- > ... > nouveau_display_hpd_work > pm_runtime_get_sync > drm_helper_hpd_irq_event > pm_runtime_mark_last_busy > pm_runtime_put_sync > > I'm curious about that "schedule_work(hpd_work)" near the end because > no other drivers seem to use schedule_work() in the runtime_resume > path, and I don't know how that synchronizes with the shutdown > process. I don't see anything that waits for > nouveau_display_hpd_work() to complete, so it seems like something > that could be a race. > > I wonder this problem would be easier to reproduce if you added a > sleep in nouveau_display_hpd_work() as in the first hunk below, and I > wonder if the problem would then go away if you stopped scheduling > hpd_work as in the second hunk? Obviously the second hunk isn't a > solution, it's just an attempt to figure out if I'm looking in the > right area. > Hi, sorry it took me so long to get back to this - I've been busy with some other responsibilities at work that came up last moment. So I did try making it so that we cancel hpd_work from pci_driver->shutdown and wait for it to complete, however that didn't really mseem to make any difference. I did however try adding a workaround in the past that would shut down the GPU whenever the kernel was shutting down (basically calling nouveau_drm_remove() on shutdown) and that did actually fix the issue though, but I didn't go with it as a final solution because of the problems it would cause if we tried shutting down the card when it's in a bad state we could end up hanging the whole system. Do we want to have this discussion on the bz btw, or is this email thread fine? > Bjorn > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c > b/drivers/gpu/drm/nouveau/nouveau_display.c > index 55c0fa451163..e50806012d41 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > @@ -350,6 +350,7 @@ nouveau_display_hpd_work(struct work_struct *work) > > pm_runtime_get_sync(drm->dev->dev); > > + msleep(2000); > drm_helper_hpd_irq_event(drm->dev); > > pm_runtime_mark_last_busy(drm->dev->dev); > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 5020265bfbd9..48da72caa017 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -946,9 +946,6 @@ nouveau_pmops_runtime_resume(struct device *dev) > nvif_mask(&device->object, 0x088488, (1 << 25), (1 << 25)); > drm_dev->switch_power_state = DRM_SWITCH_POWER_ON; > > - /* Monitors may have been connected / disconnected during suspend */ > - schedule_work(&nouveau_drm(drm_dev)->hpd_work); > - > return ret; > } >
Any update on this? This has been waiting for a while now On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote: > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > resume")]
On Wed, Apr 24, 2019 at 01:31:09PM -0400, Lyude Paul wrote: > Any update on this? This has been waiting for a while now Oh, sorry, I guess we were both waiting for the other. Let me respond to the email with context. > On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote: > > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > > resume")] > -- > Cheers, > Lyude Paul >
On Mon, Apr 15, 2019 at 02:07:18PM -0400, Lyude Paul wrote: > On Thu, 2019-04-04 at 09:17 -0500, Bjorn Helgaas wrote: > > [+cc Hans, author of 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > > resume")] > > > > On Fri, Mar 22, 2019 at 06:30:15AM -0500, Bjorn Helgaas wrote: > > > On Thu, Mar 21, 2019 at 05:48:19PM -0500, Bjorn Helgaas wrote: > > > > On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > > > > > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > > > > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > > > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > > > > > ones that come with a Quadro M1000M chip instead of the M2000M > > > > > > > > variant, the BIOS seems to have a very nasty habit of not > > > > > > > > always resetting the secondary Nvidia GPU between full reboots > > > > > > > > if the laptop is configured in Hybrid Graphics mode. The > > > > > > > > reason for this happening is unknown, but the following steps > > > > > > > > and possibly a good bit of patience will reproduce the issue: > > > > > > > > > > > > > > > > 1. Boot up the laptop normally in Hybrid graphics mode > > > > > > > > 2. Make sure nouveau is loaded and that the GPU is awake > > > > > > > > 2. Allow the nvidia GPU to runtime suspend itself after being > > > > > > > > idle > > > > > > > > 3. Reboot the machine, the more sudden the better (e.g sysrq-b > > > > > > > > may help) > > > > > > > > 4. If nouveau loads up properly, reboot the machine again and go > > > > > > > > back to > > > > > > > > step 2 until you reproduce the issue > > > > > > > > > > > > > > > > This results in some very strange behavior: the GPU will quite > > > > > > > > literally be left in exactly the same state it was in when the > > > > > > > > previously booted kernel started the reboot. This has all > > > > > > > > sorts of bad sideaffects: for starters, this completely breaks > > > > > > > > nouveau starting with a mysterious EVO channel failure that > > > > > > > > happens well before we've actually used the EVO channel for > > > > > > > > anything: > > > > > > > > Thanks for the hybrid tutorial (snipped from this response). IIUC, > > > > what you said was that in hybrid mode, the Intel GPU drives the > > > > built-in display and the Nvidia GPU drives any external displays and > > > > may be used for DRI PRIME rendering (whatever that is). But since you > > > > say the Nvidia device gets runtime suspended, I assume there's no > > > > external display here and you're not using DRI PRIME. > > > > > > > > I wonder if it's related to the fact that the Nvidia GPU has been > > > > runtime suspended before you do the reboot. Can you try turning of > > > > runtime power management for the GPU by setting the runpm module > > > > parameter to 0? I *think* this would be booting with > > > > "nouveau.runpm=0". > > > > > > Sorry, I wasn't really thinking here. You already *said* this is > > > related to runtime suspend. It only happens when the Nvidia GPU has > > > been suspended. > > > > > > I don't know that much about suspend, but ISTR seeing comments about > > > resuming devices before we shutdown. If we do that, maybe there's > > > some kind of race between that resume and the reboot? > > > > I think we do in fact resume PCI devices before shutdown. Here's the > > path I'm looking at: > > > > device_shutdown > > pm_runtime_get_noresume > > pm_runtime_barrier > > dev->bus->shutdown > > pci_device_shutdown > > pm_runtime_resume > > __pm_runtime_resume(dev, 0) > > rpm_resume(dev, 0) > > __update_runtime_status(dev, RPM_RESUMING) > > callback = RPM_GET_CALLBACK(dev, runtime_resume) > > rpm_callback(callback, dev) > > __rpm_callback > > pci_pm_runtime_resume > > drv->pm->runtime_resume > > nouveau_pmops_runtime_resume > > nouveau_do_resume > > schedule_work(hpd_work) # <--- > > ... > > nouveau_display_hpd_work > > pm_runtime_get_sync > > drm_helper_hpd_irq_event > > pm_runtime_mark_last_busy > > pm_runtime_put_sync > > > > I'm curious about that "schedule_work(hpd_work)" near the end because > > no other drivers seem to use schedule_work() in the runtime_resume > > path, and I don't know how that synchronizes with the shutdown > > process. I don't see anything that waits for > > nouveau_display_hpd_work() to complete, so it seems like something > > that could be a race. > > > > I wonder this problem would be easier to reproduce if you added a > > sleep in nouveau_display_hpd_work() as in the first hunk below, and I > > wonder if the problem would then go away if you stopped scheduling > > hpd_work as in the second hunk? Obviously the second hunk isn't a > > solution, it's just an attempt to figure out if I'm looking in the > > right area. > > So I did try making it so that we cancel hpd_work from > pci_driver->shutdown and wait for it to complete, however that > didn't really seem to make any difference. Not being a scheduled work expert, I was unsure if this experiment was equivalent to what I proposed. I'm always suspicious of singleton solutions like this (using schedule_work() in runtime_resume()) because usually they seem to be solving a generic problem that should happen on many kinds of hardware. The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) resume") commit log says: We need to call drm_helper_hpd_irq_event() on resume to properly detect monitor connection / disconnection on some laptops, use hpd_work for this to avoid deadlocks. The situation of a monitor being connected or disconnected during suspend can happen to *any* GPU, but the commit only changes nouveau, which of course raises the question of how we deal with that in other drivers. If the Nvidia GPU has some unique behavior related to monitor connection, that would explain special-case code there, but the commit doesn't mention anything like that. It should be simple to revert 0b2fe6594fa2 and see whether it changes the behavior at all (well, simple except for the fact that this problem isn't 100% reproducible in the first place). > Do we want to have this discussion on the bz btw, or is this email > thread fine? Email is fine. > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c > > b/drivers/gpu/drm/nouveau/nouveau_display.c > > index 55c0fa451163..e50806012d41 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > @@ -350,6 +350,7 @@ nouveau_display_hpd_work(struct work_struct *work) > > > > pm_runtime_get_sync(drm->dev->dev); > > > > + msleep(2000); > > drm_helper_hpd_irq_event(drm->dev); > > > > pm_runtime_mark_last_busy(drm->dev->dev); > > > > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > > b/drivers/gpu/drm/nouveau/nouveau_drm.c > > index 5020265bfbd9..48da72caa017 100644 > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > @@ -946,9 +946,6 @@ nouveau_pmops_runtime_resume(struct device *dev) > > nvif_mask(&device->object, 0x088488, (1 << 25), (1 << 25)); > > drm_dev->switch_power_state = DRM_SWITCH_POWER_ON; > > > > - /* Monitors may have been connected / disconnected during suspend */ > > - schedule_work(&nouveau_drm(drm_dev)->hpd_work); > > - > > return ret; > > } > > > -- > Cheers, > Lyude Paul >
On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote: > Not being a scheduled work expert, I was unsure if this experiment was > equivalent to what I proposed. > > I'm always suspicious of singleton solutions like this (using > schedule_work() in runtime_resume()) because usually they seem to be > solving a generic problem that should happen on many kinds of > hardware. The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > resume") commit log says: > > We need to call drm_helper_hpd_irq_event() on resume to properly > detect monitor connection / disconnection on some laptops, use > hpd_work for this to avoid deadlocks. > > The situation of a monitor being connected or disconnected during > suspend can happen to *any* GPU, but the commit only changes nouveau, > which of course raises the question of how we deal with that in other > drivers. If the Nvidia GPU has some unique behavior related to > monitor connection, that would explain special-case code there, but > the commit doesn't mention anything like that. > > It should be simple to revert 0b2fe6594fa2 and see whether it changes > the behavior at all (well, simple except for the fact that this > problem isn't 100% reproducible in the first place). It's not 100% reproducible, but it's at least 90% so it's not difficult for me to test at all. Also, reverting this commit makes no difference either. Note that while that commit only changed nouveau, scheduled_work() is exactly how a number of other drivers (i915 for instance) handle reprobing like this as well. The reason being that we can't do full connector reprobing in our runtime resume thread because we could deadlock if someone else is holding a modesetting lock we need and waiting on us to resume at the same time (there's a number of other bug fixes in nouveau for other issues caused by the same deadlock scenario). I'm confused here though, it sounds like you're running under the assumption that PCI devices like this aren't reset into a clean state during a system reboot, is that correct? > > > Do we want to have this discussion on the bz btw, or is this email > > thread fine? > > Email is fine.
On Wed, Apr 24, 2019 at 03:16:37PM -0400, Lyude Paul wrote: > On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote: > > Not being a scheduled work expert, I was unsure if this experiment was > > equivalent to what I proposed. > > > > I'm always suspicious of singleton solutions like this (using > > schedule_work() in runtime_resume()) because usually they seem to be > > solving a generic problem that should happen on many kinds of > > hardware. The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > > resume") commit log says: > > > > We need to call drm_helper_hpd_irq_event() on resume to properly > > detect monitor connection / disconnection on some laptops, use > > hpd_work for this to avoid deadlocks. > > > > The situation of a monitor being connected or disconnected during > > suspend can happen to *any* GPU, but the commit only changes nouveau, > > which of course raises the question of how we deal with that in other > > drivers. If the Nvidia GPU has some unique behavior related to > > monitor connection, that would explain special-case code there, but > > the commit doesn't mention anything like that. > > > > It should be simple to revert 0b2fe6594fa2 and see whether it changes > > the behavior at all (well, simple except for the fact that this > > problem isn't 100% reproducible in the first place). > > It's not 100% reproducible, but it's at least 90% so it's not > difficult for me to test at all. > > Also, reverting this commit makes no difference either. OK, great, that makes it crystal clear. I didn't know you had specifically tested that revert. Thanks for doing that. > Note that while that commit only changed nouveau, scheduled_work() > is exactly how a number of other drivers (i915 for instance) handle > reprobing like this as well. OK. The GPU code would be a lot more approachable if similar things were done in similar ways. I spent an hour or so looking for this similar code in i915, but gave up. > The reason being that we can't do full connector reprobing in our > runtime resume thread because we could deadlock if someone else is > holding a modesetting lock we need and waiting on us to resume at > the same time (there's a number of other bug fixes in nouveau for > other issues caused by the same deadlock scenario). You mention nouveau specifically here, but I assume this is a generic deadlock scenario that applies to any GPU, and they all avoid the deadlock in the same way. Right? > I'm confused here though, it sounds like you're running under the > assumption that PCI devices like this aren't reset into a clean > state during a system reboot, is that correct? No, I wasn't trying to say anything about that. My point here is that: - you're reporting a problem that only happens with nouveau and only happens during shutdown/reboot - the behavior is similar to a race (not 100% reproducible, seems to happen more if shutdown is faster) - shutdown involves resuming the device (see pci_device_shutdown()) - nouveau_pmops_runtime_resume() schedules asynchronous work, which (to my untrained eye) looks unusual - asynchronous work is inherently subject to races So I think that's all somewhat suspicious. But if the same problem happens without the asynchronous work, obviously the issue is elsewhere. But you *are* right that if the device were actually reset, none of this should matter. It certainly seems that the BIOS neglects to reset it in some cases. I can sort of imagine a BIOS engineer thinking that if the device looks like it's in use, we shouldn't reset it, and it's still conceivable that some sort of Linux shutdown race could leave it looking like it's in use. But you've been working with Lenovo on this, and it seems like that would be pretty obvious to somebody with the BIOS source (though I just demonstrated above that even with the source it's easy to miss things). I'm out of ideas, so I think your quirk is the best way forward. I trimmed out some of the commit log backtraces and such, added the bugzilla, and tweaked the patch to use pci_iomap() instead of ioremap(). Would the patch below work for you? commit 18dc5b3c7ddc Author: Lyude Paul <lyude@redhat.com> Date: Tue Feb 12 17:02:30 2019 -0500 PCI: Reset Lenovo ThinkPad P50 nvgpu at boot if necessary On ThinkPad P50 SKUs with an Nvidia Quadro M1000M instead of the M2000M variant, the BIOS does not always reset the secondary Nvidia GPU during reboot if the laptop is configured in Hybrid Graphics mode. The reason is unknown, but the following steps and possibly a good bit of patience will reproduce the issue: 1. Boot up the laptop normally in Hybrid Graphics mode 2. Make sure nouveau is loaded and that the GPU is awake 2. Allow the Nvidia GPU to runtime suspend itself after being idle 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) 4. If nouveau loads up properly, reboot the machine again and go back to step 2 until you reproduce the issue This results in some very strange behavior: the GPU will be left in exactly the same state it was in when the previously booted kernel started the reboot. This has all sorts of bad side effects: for starters, this completely breaks nouveau starting with a mysterious EVO channel failure that happens well before we've actually used the EVO channel for anything: nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 00000002 This causes a timeout trying to bring up the GR ctx: nouveau 0000:01:00.0: timeout WARNING: CPU: 0 PID: 12 at drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 gf100_grctx_generate+0x7b2/0x850 [nouveau] Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) 12/18/2018 Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] ... nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 engine 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 unknown] The GPU never manages to recover. Booting without loading nouveau causes issues as well, since the GPU starts sending spurious interrupts that cause other device's IRQs to get disabled by the kernel: irq 16: nobody cared (try booting with the "irqpoll" option) ... handlers: [<000000007faa9e99>] i801_isr [i2c_i801] Disabling IRQ #16 ... serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! i801_smbus 0000:00:1f.4: Transaction timeout rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX register (-110). i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! i801_smbus 0000:00:1f.4: Transaction timeout rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change enabled interrupts! This causes the touchpad and sometimes other things to get disabled. Since this happens without nouveau, we can't fix this problem from nouveau itself. Add a PCI quirk for the specific P50 variant of this GPU. Make sure the GPU is advertising NoReset- so we don't reset the GPU when the machine is in Dedicated graphics mode (where the GPU being initialized by the BIOS is normal and expected). Map the GPU MMIO space and read the magic 0x2240c register, which will have bit 1 set if the device was POSTed during a previous boot. Once we've confirmed all of this, reset the GPU and re-disable it - bringing it back to a healthy state. Link: https://bugzilla.kernel.org/show_bug.cgi?id=203003 Link: https://lore.kernel.org/lkml/20190212220230.1568-1-lyude@redhat.com Signed-off-by: Lyude Paul <lyude@redhat.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Cc: nouveau@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: Karol Herbst <kherbst@redhat.com> Cc: Ben Skeggs <skeggsb@gmail.com> Cc: stable@vger.kernel.org diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index a59ad09ce911..819a595b0b1d 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -5120,3 +5120,61 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ + +/* + * On Lenovo Thinkpad P50 SKUs with a Nvidia Quadro M1000M, the BIOS does + * not always reset the secondary Nvidia GPU between reboots if the system + * is configured to use Hybrid Graphics mode. This results in the GPU + * being left in whatever state it was in during the *previous* boot, which + * causes spurious interrupts from the GPU, which in turn causes us to + * disable the wrong IRQ and end up breaking the touchpad. Unsurprisingly, + * this also completely breaks nouveau. + * + * Luckily, it seems a simple reset of the Nvidia GPU brings it back to a + * clean state and fixes all these issues. + * + * When the machine is configured in Dedicated display mode, the issue + * doesn't occur. Fortunately the GPU advertises NoReset+ when in this + * mode, so we can detect that and avoid resetting it. + */ +static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev) +{ + void __iomem *map; + int ret; + + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || + pdev->subsystem_device != 0x222e || + !pdev->reset_fn) + return; + + if (pci_enable_device_mem(pdev)) + return; + + /* + * Based on nvkm_device_ctor() in + * drivers/gpu/drm/nouveau/nvkm/engine/device/base.c + */ + map = pci_iomap(pdev, 0, 0x23000); + if (!map) { + pci_err(pdev, "Can't map MMIO space\n"); + goto out_disable; + } + + /* + * Make sure the GPU looks like it's been POSTed before resetting + * it. + */ + if (ioread32(map + 0x2240c) & 0x2) { + pci_info(pdev, FW_BUG "GPU left initialized by EFI, resetting\n"); + ret = pci_reset_function(pdev); + if (ret < 0) + pci_err(pdev, "Failed to reset GPU: %d\n", ret); + } + + iounmap(map); +out_disable: + pci_disable_device(pdev); +} +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, + PCI_CLASS_DISPLAY_VGA, 8, + quirk_reset_lenovo_thinkpad_p50_nvgpu);
On Wed, 2019-04-24 at 17:36 -0500, Bjorn Helgaas wrote: > On Wed, Apr 24, 2019 at 03:16:37PM -0400, Lyude Paul wrote: > > On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote: > > > Not being a scheduled work expert, I was unsure if this experiment was > > > equivalent to what I proposed. > > > > > > I'm always suspicious of singleton solutions like this (using > > > schedule_work() in runtime_resume()) because usually they seem to be > > > solving a generic problem that should happen on many kinds of > > > hardware. The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > > > resume") commit log says: > > > > > > We need to call drm_helper_hpd_irq_event() on resume to properly > > > detect monitor connection / disconnection on some laptops, use > > > hpd_work for this to avoid deadlocks. > > > > > > The situation of a monitor being connected or disconnected during > > > suspend can happen to *any* GPU, but the commit only changes nouveau, > > > which of course raises the question of how we deal with that in other > > > drivers. If the Nvidia GPU has some unique behavior related to > > > monitor connection, that would explain special-case code there, but > > > the commit doesn't mention anything like that. > > > > > > It should be simple to revert 0b2fe6594fa2 and see whether it changes > > > the behavior at all (well, simple except for the fact that this > > > problem isn't 100% reproducible in the first place). > > > > It's not 100% reproducible, but it's at least 90% so it's not > > difficult for me to test at all. > > > > Also, reverting this commit makes no difference either. > > OK, great, that makes it crystal clear. I didn't know you had > specifically tested that revert. Thanks for doing that. > > > Note that while that commit only changed nouveau, scheduled_work() > > is exactly how a number of other drivers (i915 for instance) handle > > reprobing like this as well. > > OK. The GPU code would be a lot more approachable if similar things > were done in similar ways. I spent an hour or so looking for this > similar code in i915, but gave up. We try > > > The reason being that we can't do full connector reprobing in our > > runtime resume thread because we could deadlock if someone else is > > holding a modesetting lock we need and waiting on us to resume at > > the same time (there's a number of other bug fixes in nouveau for > > other issues caused by the same deadlock scenario). > > You mention nouveau specifically here, but I assume this is a generic > deadlock scenario that applies to any GPU, and they all avoid the > deadlock in the same way. Right? Yes-there is only one exception in the tree that I know of (amdgpu/radeon), but fixing that is on my todo if someone else hasn't already gotten to it yet. > > > I'm confused here though, it sounds like you're running under the > > assumption that PCI devices like this aren't reset into a clean > > state during a system reboot, is that correct? > > No, I wasn't trying to say anything about that. My point here is > that: > > - you're reporting a problem that only happens with nouveau and > only happens during shutdown/reboot > - the behavior is similar to a race (not 100% reproducible, seems > to happen more if shutdown is faster) > - shutdown involves resuming the device (see pci_device_shutdown()) > - nouveau_pmops_runtime_resume() schedules asynchronous work, which > (to my untrained eye) looks unusual > - asynchronous work is inherently subject to races > > So I think that's all somewhat suspicious. But if the same problem > happens without the asynchronous work, obviously the issue is > elsewhere. > > But you *are* right that if the device were actually reset, none of > this should matter. It certainly seems that the BIOS neglects to > reset it in some cases. > > I can sort of imagine a BIOS engineer thinking that if the device > looks like it's in use, we shouldn't reset it, and it's still > conceivable that some sort of Linux shutdown race could leave it > looking like it's in use. But you've been working with Lenovo on > this, and it seems like that would be pretty obvious to somebody with > the BIOS source (though I just demonstrated above that even with the > source it's easy to miss things). Yeah, that's what I thought as well! This experience has taught me that there's a surprising amount of things about BIOSes that BIOS engineers don't seem to know about… > > I'm out of ideas, so I think your quirk is the best way forward. I > trimmed out some of the commit log backtraces and such, added the > bugzilla, and tweaked the patch to use pci_iomap() instead of > ioremap(). Would the patch below work for you? Works for me, tested on the problematic P50 as well. > > > commit 18dc5b3c7ddc > Author: Lyude Paul <lyude@redhat.com> > Date: Tue Feb 12 17:02:30 2019 -0500 > > PCI: Reset Lenovo ThinkPad P50 nvgpu at boot if necessary > > On ThinkPad P50 SKUs with an Nvidia Quadro M1000M instead of the M2000M > variant, the BIOS does not always reset the secondary Nvidia GPU during > reboot if the laptop is configured in Hybrid Graphics mode. The reason > is > unknown, but the following steps and possibly a good bit of patience > will > reproduce the issue: > > 1. Boot up the laptop normally in Hybrid Graphics mode > 2. Make sure nouveau is loaded and that the GPU is awake > 2. Allow the Nvidia GPU to runtime suspend itself after being idle > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may > help) > 4. If nouveau loads up properly, reboot the machine again and go back > to > step 2 until you reproduce the issue > > This results in some very strange behavior: the GPU will be left in > exactly > the same state it was in when the previously booted kernel started the > reboot. This has all sorts of bad side effects: for starters, this > completely breaks nouveau starting with a mysterious EVO channel failure > that happens well before we've actually used the EVO channel for > anything: > > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > 00000002 > > This causes a timeout trying to bring up the GR ctx: > > nouveau 0000:01:00.0: timeout > WARNING: CPU: 0 PID: 12 at > drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 > gf100_grctx_generate+0x7b2/0x850 [nouveau] > Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) > 12/18/2018 > Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] > ... > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, > busy: 1) > nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, > busy: 1) > nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 > engine 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 > unknown] > > The GPU never manages to recover. Booting without loading nouveau > causes > issues as well, since the GPU starts sending spurious interrupts that > cause > other device's IRQs to get disabled by the kernel: > > irq 16: nobody cared (try booting with the "irqpoll" option) > ... > handlers: > [<000000007faa9e99>] i801_isr [i2c_i801] > Disabling IRQ #16 > ... > serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > i801_smbus 0000:00:1f.4: Transaction timeout > rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX > register (-110). > i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! > i801_smbus 0000:00:1f.4: Transaction timeout > rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change > enabled interrupts! > > This causes the touchpad and sometimes other things to get disabled. > > Since this happens without nouveau, we can't fix this problem from > nouveau > itself. > > Add a PCI quirk for the specific P50 variant of this GPU. Make sure the > GPU is advertising NoReset- so we don't reset the GPU when the machine > is > in Dedicated graphics mode (where the GPU being initialized by the BIOS > is > normal and expected). Map the GPU MMIO space and read the magic 0x2240c > register, which will have bit 1 set if the device was POSTed during a > previous boot. Once we've confirmed all of this, reset the GPU and > re-disable it - bringing it back to a healthy state. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=203003 > Link: > https://lore.kernel.org/lkml/20190212220230.1568-1-lyude@redhat.com > Signed-off-by: Lyude Paul <lyude@redhat.com> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Cc: nouveau@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: Karol Herbst <kherbst@redhat.com> > Cc: Ben Skeggs <skeggsb@gmail.com> > Cc: stable@vger.kernel.org > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index a59ad09ce911..819a595b0b1d 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5120,3 +5120,61 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > + > +/* > + * On Lenovo Thinkpad P50 SKUs with a Nvidia Quadro M1000M, the BIOS does > + * not always reset the secondary Nvidia GPU between reboots if the system > + * is configured to use Hybrid Graphics mode. This results in the GPU > + * being left in whatever state it was in during the *previous* boot, which > + * causes spurious interrupts from the GPU, which in turn causes us to > + * disable the wrong IRQ and end up breaking the touchpad. Unsurprisingly, > + * this also completely breaks nouveau. > + * > + * Luckily, it seems a simple reset of the Nvidia GPU brings it back to a > + * clean state and fixes all these issues. > + * > + * When the machine is configured in Dedicated display mode, the issue > + * doesn't occur. Fortunately the GPU advertises NoReset+ when in this > + * mode, so we can detect that and avoid resetting it. > + */ > +static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev) > +{ > + void __iomem *map; > + int ret; > + > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > + pdev->subsystem_device != 0x222e || > + !pdev->reset_fn) > + return; > + > + if (pci_enable_device_mem(pdev)) > + return; > + > + /* > + * Based on nvkm_device_ctor() in > + * drivers/gpu/drm/nouveau/nvkm/engine/device/base.c > + */ > + map = pci_iomap(pdev, 0, 0x23000); > + if (!map) { > + pci_err(pdev, "Can't map MMIO space\n"); > + goto out_disable; > + } > + > + /* > + * Make sure the GPU looks like it's been POSTed before resetting > + * it. > + */ > + if (ioread32(map + 0x2240c) & 0x2) { > + pci_info(pdev, FW_BUG "GPU left initialized by EFI, > resetting\n"); > + ret = pci_reset_function(pdev); > + if (ret < 0) > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > + } > + > + iounmap(map); > +out_disable: > + pci_disable_device(pdev); > +} > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > + PCI_CLASS_DISPLAY_VGA, 8, > + quirk_reset_lenovo_thinkpad_p50_nvgpu);
On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > On a very specific subset of ThinkPad P50 SKUs, particularly ones that > come with a Quadro M1000M chip instead of the M2000M variant, the BIOS > seems to have a very nasty habit of not always resetting the secondary > Nvidia GPU between full reboots if the laptop is configured in Hybrid > Graphics mode. The reason for this happening is unknown, but the > following steps and possibly a good bit of patience will reproduce the > issue: > > 1. Boot up the laptop normally in Hybrid graphics mode > 2. Make sure nouveau is loaded and that the GPU is awake > 2. Allow the nvidia GPU to runtime suspend itself after being idle > 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) > 4. If nouveau loads up properly, reboot the machine again and go back to > step 2 until you reproduce the issue > > This results in some very strange behavior: the GPU will > quite literally be left in exactly the same state it was in when the > previously booted kernel started the reboot. This has all sorts of bad > sideaffects: for starters, this completely breaks nouveau starting with a > mysterious EVO channel failure that happens well before we've actually > used the EVO channel for anything: > > nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 > 00000002 > ... > So to do this, we add a new pci quirk using > DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe > at boot finishes. From there, we check to make sure that this is indeed > the specific P50 variant of this GPU. We also make sure that the GPU PCI > device is advertising NoReset- in order to prevent us from trying to > reset the GPU when the machine is in Dedicated graphics mode (where the > GPU being initialized by the BIOS is normal and expected). Finally, we > try mapping the MMIO space for the GPU which should only work if the GPU > is actually active in D0 mode. We can then read the magic 0x2240c > register on the GPU, which will have bit 1 set if the GPU's firmware has > already been posted during a previous boot. Once we've confirmed all of > this, we reset the PCI device and re-disable it - bringing the GPU back > into a healthy state. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: nouveau@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: Karol Herbst <kherbst@redhat.com> > Cc: Ben Skeggs <skeggsb@gmail.com> > Cc: stable@vger.kernel.org Applied to pci/misc for v5.2, thanks! > --- > drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index b0a413f3f7ca..948492fda8bf 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ > SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ > SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ > SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ > + > +/* > + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a Nvidia > + * Quadro M1000M, the BIOS will occasionally make the mistake of not resetting > + * the nvidia GPU between reboots if the system is configured to use hybrid > + * graphics mode. This results in the GPU being left in whatever state it was > + * in during the previous boot which causes spurious interrupts from the GPU, > + * which in turn cause us to disable the wrong IRQs and end up breaking the > + * touchpad. Unsurprisingly, this also completely breaks nouveau. > + * > + * Luckily, it seems a simple reset of the PCI device for the nvidia GPU > + * manages to bring the GPU back into a clean state and fix all of these > + * issues. Additionally since the GPU will report NoReset+ when the machine is > + * configured in Dedicated display mode, we don't need to worry about > + * accidentally resetting the GPU when it's supposed to already be > + * initialized. > + */ > +static void > +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) > +{ > + void __iomem *map; > + int ret; > + > + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || > + pdev->subsystem_device != 0x222e || > + !pdev->reset_fn) > + return; > + > + /* > + * If we can't enable the device's mmio space, it's probably not even > + * initialized. This is fine, and means we can just skip the quirk > + * entirely. > + */ > + if (pci_enable_device_mem(pdev)) { > + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); > + return; > + } > + > + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ > + map = ioremap(pci_resource_start(pdev, 0), 0x102000); > + if (!map) { > + pci_err(pdev, "Can't map MMIO space, this is probably very bad\n"); > + goto out_disable; > + } > + > + /* > + * Be extra careful, and make sure that the GPU firmware is posted > + * before trying a reset > + */ > + if (ioread32(map + 0x2240c) & 0x2) { > + pci_info(pdev, > + FW_BUG "GPU left initialized by EFI, resetting\n"); > + ret = pci_reset_function(pdev); > + if (ret < 0) > + pci_err(pdev, "Failed to reset GPU: %d\n", ret); > + } > + > + iounmap(map); > +out_disable: > + pci_disable_device(pdev); > +} > + > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, > + PCI_CLASS_DISPLAY_VGA, 8, > + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot); > -- > 2.20.1 >
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index b0a413f3f7ca..948492fda8bf 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -5117,3 +5117,68 @@ SWITCHTEC_QUIRK(0x8573); /* PFXI 48XG3 */ SWITCHTEC_QUIRK(0x8574); /* PFXI 64XG3 */ SWITCHTEC_QUIRK(0x8575); /* PFXI 80XG3 */ SWITCHTEC_QUIRK(0x8576); /* PFXI 96XG3 */ + +/* + * On certain Lenovo Thinkpad P50 SKUs, specifically those with a Nvidia + * Quadro M1000M, the BIOS will occasionally make the mistake of not resetting + * the nvidia GPU between reboots if the system is configured to use hybrid + * graphics mode. This results in the GPU being left in whatever state it was + * in during the previous boot which causes spurious interrupts from the GPU, + * which in turn cause us to disable the wrong IRQs and end up breaking the + * touchpad. Unsurprisingly, this also completely breaks nouveau. + * + * Luckily, it seems a simple reset of the PCI device for the nvidia GPU + * manages to bring the GPU back into a clean state and fix all of these + * issues. Additionally since the GPU will report NoReset+ when the machine is + * configured in Dedicated display mode, we don't need to worry about + * accidentally resetting the GPU when it's supposed to already be + * initialized. + */ +static void +quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot(struct pci_dev *pdev) +{ + void __iomem *map; + int ret; + + if (pdev->subsystem_vendor != PCI_VENDOR_ID_LENOVO || + pdev->subsystem_device != 0x222e || + !pdev->reset_fn) + return; + + /* + * If we can't enable the device's mmio space, it's probably not even + * initialized. This is fine, and means we can just skip the quirk + * entirely. + */ + if (pci_enable_device_mem(pdev)) { + pci_dbg(pdev, "Can't enable device mem, no reset needed\n"); + return; + } + + /* Taken from drivers/gpu/drm/nouveau/engine/device/base.c */ + map = ioremap(pci_resource_start(pdev, 0), 0x102000); + if (!map) { + pci_err(pdev, "Can't map MMIO space, this is probably very bad\n"); + goto out_disable; + } + + /* + * Be extra careful, and make sure that the GPU firmware is posted + * before trying a reset + */ + if (ioread32(map + 0x2240c) & 0x2) { + pci_info(pdev, + FW_BUG "GPU left initialized by EFI, resetting\n"); + ret = pci_reset_function(pdev); + if (ret < 0) + pci_err(pdev, "Failed to reset GPU: %d\n", ret); + } + + iounmap(map); +out_disable: + pci_disable_device(pdev); +} + +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1, + PCI_CLASS_DISPLAY_VGA, 8, + quirk_lenovo_thinkpad_p50_nvgpu_survives_reboot);
On a very specific subset of ThinkPad P50 SKUs, particularly ones that come with a Quadro M1000M chip instead of the M2000M variant, the BIOS seems to have a very nasty habit of not always resetting the secondary Nvidia GPU between full reboots if the laptop is configured in Hybrid Graphics mode. The reason for this happening is unknown, but the following steps and possibly a good bit of patience will reproduce the issue: 1. Boot up the laptop normally in Hybrid graphics mode 2. Make sure nouveau is loaded and that the GPU is awake 2. Allow the nvidia GPU to runtime suspend itself after being idle 3. Reboot the machine, the more sudden the better (e.g sysrq-b may help) 4. If nouveau loads up properly, reboot the machine again and go back to step 2 until you reproduce the issue This results in some very strange behavior: the GPU will quite literally be left in exactly the same state it was in when the previously booted kernel started the reboot. This has all sorts of bad sideaffects: for starters, this completely breaks nouveau starting with a mysterious EVO channel failure that happens well before we've actually used the EVO channel for anything: nouveau 0000:01:00.0: disp: chid 0 mthd 0000 data 00000400 00001000 00000002 Later on, this causes us to timeout trying to bring up the GR ctx: ------------[ cut here ]------------ nouveau 0000:01:00.0: timeout WARNING: CPU: 0 PID: 12 at drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 gf100_grctx_generate+0x7b2/0x850 [nouveau] Modules linked in: nouveau mxm_wmi i915 crc32c_intel ttm i2c_algo_bit serio_raw drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops xhci_pci drm xhci_hcd i2c_core wmi video CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc5Lyude-Test+ #29 Hardware name: LENOVO 20EQS64N0B/20EQS64N0B, BIOS N1EET82W (1.55 ) 12/18/2018 Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper] RIP: 0010:gf100_grctx_generate+0x7b2/0x850 [nouveau] Code: 85 d2 75 04 48 8b 57 10 48 89 95 28 ff ff ff e8 b4 37 0e e1 48 8b 95 28 ff ff ff 48 c7 c7 b1 97 57 a0 48 89 c6 e8 5a 38 c0 e0 <0f> 0b e9 b9 fd ff ff 48 8b 85 60 ff ff ff 48 8b 40 10 48 8b 78 10 RSP: 0018:ffffc900000b77f0 EFLAGS: 00010286 RAX: 0000000000000000 RBX: ffff888871af8000 RCX: 0000000000000000 RDX: ffff88887f41dfe0 RSI: ffff88887f415698 RDI: ffff88887f415698 RBP: ffffc900000b78c8 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: ffff888872118000 R13: 0000000000000000 R14: ffffffffa0551420 R15: ffffc900000b7818 FS: 0000000000000000(0000) GS:ffff88887f400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00005644d0556ca8 CR3: 0000000002214006 CR4: 00000000003606f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: gf100_gr_init_ctxctl+0x27b/0x2d0 [nouveau] gf100_gr_init+0x5bd/0x5e0 [nouveau] gf100_gr_init_+0x61/0x70 [nouveau] nvkm_gr_init+0x1d/0x20 [nouveau] nvkm_engine_init+0xcb/0x210 [nouveau] nvkm_subdev_init+0xd6/0x230 [nouveau] nvkm_engine_ref.part.0+0x52/0x70 [nouveau] nvkm_engine_ref+0x13/0x20 [nouveau] nvkm_ioctl_new+0x12c/0x260 [nouveau] ? nvkm_fifo_chan_child_del+0xa0/0xa0 [nouveau] ? gf100_gr_dtor+0xe0/0xe0 [nouveau] nvkm_ioctl+0xe2/0x180 [nouveau] nvkm_client_ioctl+0x12/0x20 [nouveau] nvif_object_ioctl+0x47/0x50 [nouveau] nvif_object_init+0xc8/0x120 [nouveau] nvc0_fbcon_accel_init+0x5c/0x960 [nouveau] nouveau_fbcon_create+0x5a5/0x5d0 [nouveau] ? drm_setup_crtcs+0x27b/0xcb0 [drm_kms_helper] ? __lock_is_held+0x5e/0xa0 __drm_fb_helper_initial_config_and_unlock+0x27c/0x520 [drm_kms_helper] drm_fb_helper_hotplug_event.part.29+0xae/0xc0 [drm_kms_helper] drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper] nouveau_fbcon_output_poll_changed+0xb8/0x110 [nouveau] drm_kms_helper_hotplug_event+0x2a/0x40 [drm_kms_helper] drm_dp_send_link_address+0x176/0x1c0 [drm_kms_helper] drm_dp_check_and_send_link_address+0xa0/0xb0 [drm_kms_helper] drm_dp_mst_link_probe_work+0xa4/0xc0 [drm_kms_helper] process_one_work+0x22f/0x5c0 worker_thread+0x44/0x3a0 kthread+0x12b/0x150 ? wq_pool_ids_show+0x140/0x140 ? kthread_create_on_node+0x60/0x60 ret_from_fork+0x3a/0x50 irq event stamp: 22490 hardirqs last enabled at (22489): [<ffffffff8113281d>] console_unlock+0x44d/0x5f0 hardirqs last disabled at (22490): [<ffffffff81001c03>] trace_hardirqs_off_thunk+0x1a/0x1c softirqs last enabled at (22486): [<ffffffff81c00330>] __do_softirq+0x330/0x44d softirqs last disabled at (22479): [<ffffffff810c3105>] irq_exit+0xe5/0xf0 WARNING: CPU: 0 PID: 12 at drivers/gpu/drm/nouveau/nvkm/engine/gr/ctxgf100.c:1547 gf100_grctx_generate+0x7b2/0x850 [nouveau] ---[ end trace bf0976ed88b122a8 ]--- nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) nouveau 0000:01:00.0: gr: wait for idle timeout (en: 1, ctxsw: 0, busy: 1) nouveau 0000:01:00.0: fifo: fault 01 [WRITE] at 0000000000008000 engine 00 [GR] client 15 [HUB/SCC_NB] reason c4 [] on channel -1 [0000000000 unknown] From which the GPU never manages to recover. Booting without nouveau loading causes issues as well, since the GPU starts sending spurious interrupts that cause other device's IRQs to get disabled by the kernel: irq 16: nobody cared (try booting with the "irqpoll" option) … handlers: [<000000007faa9e99>] i801_isr [i2c_i801] Disabling IRQ #16 … serio: RMI4 PS/2 pass-through port at rmi4-00.fn03 i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! i801_smbus 0000:00:1f.4: Transaction timeout rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03 TX register (-110). i801_smbus 0000:00:1f.4: Timeout waiting for interrupt! i801_smbus 0000:00:1f.4: Transaction timeout rmi4_physical rmi4-00: rmi_driver_set_irq_bits: Failed to change enabled interrupts! Which in turn causes the touchpad and sometimes even other things to get disabled. Since the GPU staying on causes problems even without nouveau's intervention, we can't fix this problem from nouveau itself. We have to fix it as early as possible in the boot sequence in order to make sure that the GPU is in a clean state before it has a chance to spam us with interrupts and break things. So to do this, we add a new pci quirk using DECLARE_PCI_FIXUP_CLASS_FINAL that will be invoked before the PCI probe at boot finishes. From there, we check to make sure that this is indeed the specific P50 variant of this GPU. We also make sure that the GPU PCI device is advertising NoReset- in order to prevent us from trying to reset the GPU when the machine is in Dedicated graphics mode (where the GPU being initialized by the BIOS is normal and expected). Finally, we try mapping the MMIO space for the GPU which should only work if the GPU is actually active in D0 mode. We can then read the magic 0x2240c register on the GPU, which will have bit 1 set if the GPU's firmware has already been posted during a previous boot. Once we've confirmed all of this, we reset the PCI device and re-disable it - bringing the GPU back into a healthy state. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: nouveau@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: Karol Herbst <kherbst@redhat.com> Cc: Ben Skeggs <skeggsb@gmail.com> Cc: stable@vger.kernel.org --- drivers/pci/quirks.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)