linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
To: linux-pci@vger.kernel.org
Cc: hi-angel@yandex.ru, helgaas@kernel.org, lukas@wunner.de,
	rjw@rjwysocki.net, andreas.noever@gmail.com
Subject: [PATCH v2] PCI: don't call firmware hooks on suspend unless it's fw-controlled
Date: Fri, 21 May 2021 02:55:01 +0300	[thread overview]
Message-ID: <20210520235501.917397-1-Hi-Angel@yandex.ru> (raw)
In-Reply-To: <20210520194935.GA348608@bjorn-Precision-5520>

On Macbook 2013 resuming from s2idle resulted in external monitor no
longer being detected, and dmesg having errors like:

    pcieport 0000:06:00.0: can't change power state from D3hot to D0 (config space inaccessible)

and a stacktrace. The reason is that in s2idle (and in S1 as noted by
Rafael) we do not call firmware code to handle suspend, and as result
while waking up firmware also does not handle resume.

This means, for the Thunderbolt controller that gets disabled in the
quirk by calling the firmware methods, there's no one to wake it back up
on resume.

To quote Rafael Wysocki:

> "Passing control to the platform firmware" means letting
> some native firmware code (like SMM code) run which happens at the end
> of S2/S3/S4 suspend transitions and it does not happen during S1
> (standby) and s2idle suspend transitions.
>
> That's why using SXIO/SXFP/SXLF is only valid during S2/S3/S4 suspend
> transitions and it is not valid during s2idle and S1 suspend
> transitions (and yes, S1 is also affected, so s2idle is not special in
> that respect at all).

Thus, return early from the quirk when suspend mode isn't one that calls
firmware.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212767
Signed-off-by: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/quirks.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e3ba9e..f86b6388a04a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -27,6 +27,7 @@
 #include <linux/nvme.h>
 #include <linux/platform_data/x86/apple.h>
 #include <linux/pm_runtime.h>
+#include <linux/suspend.h>
 #include <linux/switchtec.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
 #include "pci.h"
@@ -3646,6 +3647,15 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
 		return;
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM)
 		return;
+
+	/*
+	 * SXIO/SXFP/SXLF turns off power to the Thunderbolt controller.  We don't
+	 * know how to turn it back on again, but firmware does, so we can only use
+	 * SXIO/SXFP/SXLF if we're suspending via firmware.
+	 */
+	if (!pm_suspend_via_firmware())
+		return;
+
 	bridge = ACPI_HANDLE(&dev->dev);
 	if (!bridge)
 		return;
-- 
2.31.1


  parent reply	other threads:[~2021-05-20 23:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06 17:38 [PATCH] PCI: don't power-off apple thunderbolt controller on s2idle Konstantin Kharlamov
2021-05-06 21:48 ` Bjorn Helgaas
2021-05-06 22:07   ` Lukas Wunner
2021-05-07  9:51     ` Rafael J. Wysocki
2021-05-08  8:48       ` Lukas Wunner
2021-05-07 13:30     ` Bjorn Helgaas
2021-05-07 14:08       ` Konstantin Kharlamov
2021-05-12 20:36         ` Konstantin Kharlamov
2021-05-17 19:51           ` PING " Konstantin Kharlamov
2021-05-19 17:28         ` Bjorn Helgaas
2021-05-19 19:12           ` Rafael J. Wysocki
2021-05-19 19:48             ` Bjorn Helgaas
2021-05-20 11:27               ` Rafael J. Wysocki
2021-05-20 11:54                 ` Rafael J. Wysocki
2021-05-20 19:49                   ` Bjorn Helgaas
2021-05-20 23:28                     ` Konstantin Kharlamov
2021-05-24  6:59                       ` Konstantin Kharlamov
2021-05-20 23:55                     ` Konstantin Kharlamov [this message]
2021-05-28  7:39                       ` PING: Re: [PATCH v2] PCI: don't call firmware hooks on suspend unless it's fw-controlled Konstantin Kharlamov
2021-06-03  8:36                         ` PING: " Konstantin Kharlamov
2021-06-03 17:46                           ` Bjorn Helgaas
2021-06-04  8:30                             ` Konstantin Kharlamov
2021-05-21  9:47                     ` [PATCH] PCI: don't power-off apple thunderbolt controller on s2idle Rafael J. Wysocki
2021-05-07 15:02       ` Rafael J. Wysocki
2021-05-08  8:20       ` Lukas Wunner
2021-05-07  9:32   ` Konstantin Kharlamov
2021-05-07 13:07     ` Bjorn Helgaas
2021-05-07 13:48       ` Konstantin Kharlamov
2021-05-20 11:58   ` Rafael J. Wysocki
2021-06-07 23:17 ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210520235501.917397-1-Hi-Angel@yandex.ru \
    --to=hi-angel@yandex.ru \
    --cc=andreas.noever@gmail.com \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).