On Fri, 2021-05-21 at 02:55 +0300, Konstantin Kharlamov wrote: > 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@xxxxxxxxx> > Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx> > --- > 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;
