Cool, I see, thank you very much! On Thu, 2021-06-03 at 12:46 -0500, Bjorn Helgaas wrote: > On Thu, Jun 03, 2021 at 11:36:43AM +0300, Konstantin Kharlamov wrote: > > On Fri, 2021-05-28 at 10:39 +0300, Konstantin Kharlamov wrote: > > > 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; > > > > > > > > > > > > Don't worry, I haven't forgotten, but I've been busy with some other > patches. If you ever want to check on the status, you can search for > the patch on https://patchwork.kernel.org/project/linux-pci/list. ; The > patchwork search is not super convenient (it's buried in the "Show > patches with" link), but here's your patch: > > https://patchwork.kernel.org/project/linux-pci/patch/20210520235501.917397-1-Hi-Angel@xxxxxxxxx/ > > It's currently "New" which means it's still in my queue. I change the > state to "Accepted," "Not Applicable," "Superseded," etc., when I > apply or drop patches.
