* [PATCH] PCI/PM: Skip link training delay for S3 resume
@ 2020-03-11 4:52 Kai-Heng Feng
2020-03-11 10:28 ` Mika Westerberg
0 siblings, 1 reply; 6+ messages in thread
From: Kai-Heng Feng @ 2020-03-11 4:52 UTC (permalink / raw)
To: bhelgaas; +Cc: mika.westerberg, linux-pci, linux-kernel, Kai-Heng Feng
Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the
PCIe spec") added a 1100ms delay on resume for bridges that don't
support Link Active Reporting.
The commit also states that the delay can be skipped for S3, as the
firmware should already handled the case for us.
So let's skip the link training delay for S3, to save 1100ms resume
time.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/pci/pci-driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 0454ca0e4e3f..3050375bad04 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev)
pci_fixup_device(pci_fixup_resume_early, pci_dev);
pcie_pme_root_status_cleanup(pci_dev);
- if (!skip_bus_pm && prev_state == PCI_D3cold)
+ if (!skip_bus_pm && prev_state == PCI_D3cold
+ && !pm_resume_via_firmware())
pci_bridge_wait_for_secondary_bus(pci_dev);
if (pci_has_legacy_pm_support(pci_dev))
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] PCI/PM: Skip link training delay for S3 resume 2020-03-11 4:52 [PATCH] PCI/PM: Skip link training delay for S3 resume Kai-Heng Feng @ 2020-03-11 10:28 ` Mika Westerberg 2020-03-12 4:23 ` Kai-Heng Feng 0 siblings, 1 reply; 6+ messages in thread From: Mika Westerberg @ 2020-03-11 10:28 UTC (permalink / raw) To: Kai-Heng Feng; +Cc: bhelgaas, linux-pci, linux-kernel Hi, On Wed, Mar 11, 2020 at 12:52:49PM +0800, Kai-Heng Feng wrote: > Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the > PCIe spec") added a 1100ms delay on resume for bridges that don't > support Link Active Reporting. > > The commit also states that the delay can be skipped for S3, as the > firmware should already handled the case for us. Delay can be skipped if the firmware provides _DSM with function 8 implemented according to PCI firmwre spec 3.2 sec 4.6.8. > So let's skip the link training delay for S3, to save 1100ms resume > time. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/pci/pci-driver.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 0454ca0e4e3f..3050375bad04 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev) > pci_fixup_device(pci_fixup_resume_early, pci_dev); > pcie_pme_root_status_cleanup(pci_dev); > > - if (!skip_bus_pm && prev_state == PCI_D3cold) > + if (!skip_bus_pm && prev_state == PCI_D3cold > + && !pm_resume_via_firmware()) So this would need to check for the _DSM result as well. We do evaluate it in pci_acpi_optimize_delay() (drivers/pci/pci-acpi.c) and that ends up lowering ->d3cold_delay so maybe check that here. > pci_bridge_wait_for_secondary_bus(pci_dev); > > if (pci_has_legacy_pm_support(pci_dev)) > -- > 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/PM: Skip link training delay for S3 resume 2020-03-11 10:28 ` Mika Westerberg @ 2020-03-12 4:23 ` Kai-Heng Feng 2020-03-12 8:04 ` Mika Westerberg 0 siblings, 1 reply; 6+ messages in thread From: Kai-Heng Feng @ 2020-03-12 4:23 UTC (permalink / raw) To: Mika Westerberg; +Cc: bhelgaas, linux-pci, linux-kernel Hi, > On Mar 11, 2020, at 18:28, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Wed, Mar 11, 2020 at 12:52:49PM +0800, Kai-Heng Feng wrote: >> Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the >> PCIe spec") added a 1100ms delay on resume for bridges that don't >> support Link Active Reporting. >> >> The commit also states that the delay can be skipped for S3, as the >> firmware should already handled the case for us. > > Delay can be skipped if the firmware provides _DSM with function 8 > implemented according to PCI firmwre spec 3.2 sec 4.6.8. As someone who doesn't have access to the PCI spec... Questions below. > >> So let's skip the link training delay for S3, to save 1100ms resume >> time. >> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> --- >> drivers/pci/pci-driver.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> index 0454ca0e4e3f..3050375bad04 100644 >> --- a/drivers/pci/pci-driver.c >> +++ b/drivers/pci/pci-driver.c >> @@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev) >> pci_fixup_device(pci_fixup_resume_early, pci_dev); >> pcie_pme_root_status_cleanup(pci_dev); >> >> - if (!skip_bus_pm && prev_state == PCI_D3cold) >> + if (!skip_bus_pm && prev_state == PCI_D3cold >> + && !pm_resume_via_firmware()) > > So this would need to check for the _DSM result as well. We do evaluate > it in pci_acpi_optimize_delay() (drivers/pci/pci-acpi.c) and that ends > up lowering ->d3cold_delay so maybe check that here. Do we need to wait for d3cold_delay here? Or we can also skip that as long as pci_acpi_dsm_guid and FUNCTION_DELAY_DSM present? Kai-Heng > >> pci_bridge_wait_for_secondary_bus(pci_dev); >> >> if (pci_has_legacy_pm_support(pci_dev)) >> -- >> 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/PM: Skip link training delay for S3 resume 2020-03-12 4:23 ` Kai-Heng Feng @ 2020-03-12 8:04 ` Mika Westerberg 2020-03-12 9:45 ` Kai-Heng Feng 0 siblings, 1 reply; 6+ messages in thread From: Mika Westerberg @ 2020-03-12 8:04 UTC (permalink / raw) To: Kai-Heng Feng; +Cc: bhelgaas, linux-pci, linux-kernel On Thu, Mar 12, 2020 at 12:23:46PM +0800, Kai-Heng Feng wrote: > Hi, > > > On Mar 11, 2020, at 18:28, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > > > Hi, > > > > On Wed, Mar 11, 2020 at 12:52:49PM +0800, Kai-Heng Feng wrote: > >> Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the > >> PCIe spec") added a 1100ms delay on resume for bridges that don't > >> support Link Active Reporting. > >> > >> The commit also states that the delay can be skipped for S3, as the > >> firmware should already handled the case for us. > > > > Delay can be skipped if the firmware provides _DSM with function 8 > > implemented according to PCI firmwre spec 3.2 sec 4.6.8. > > As someone who doesn't have access to the PCI spec... > Questions below. > > > > >> So let's skip the link training delay for S3, to save 1100ms resume > >> time. > >> > >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > >> --- > >> drivers/pci/pci-driver.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > >> index 0454ca0e4e3f..3050375bad04 100644 > >> --- a/drivers/pci/pci-driver.c > >> +++ b/drivers/pci/pci-driver.c > >> @@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev) > >> pci_fixup_device(pci_fixup_resume_early, pci_dev); > >> pcie_pme_root_status_cleanup(pci_dev); > >> > >> - if (!skip_bus_pm && prev_state == PCI_D3cold) > >> + if (!skip_bus_pm && prev_state == PCI_D3cold > >> + && !pm_resume_via_firmware()) > > > > So this would need to check for the _DSM result as well. We do evaluate > > it in pci_acpi_optimize_delay() (drivers/pci/pci-acpi.c) and that ends > > up lowering ->d3cold_delay so maybe check that here. > > Do we need to wait for d3cold_delay here? > Or we can also skip that as long as pci_acpi_dsm_guid and FUNCTION_DELAY_DSM present? Actually I think pci_bridge_wait_for_secondary_bus() already takes it into account. Have you checked if the BIOS has this _DSM implemented in the first place? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/PM: Skip link training delay for S3 resume 2020-03-12 8:04 ` Mika Westerberg @ 2020-03-12 9:45 ` Kai-Heng Feng 2020-03-12 10:08 ` Mika Westerberg 0 siblings, 1 reply; 6+ messages in thread From: Kai-Heng Feng @ 2020-03-12 9:45 UTC (permalink / raw) To: Mika Westerberg; +Cc: bhelgaas, linux-pci, linux-kernel > On Mar 12, 2020, at 16:04, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > On Thu, Mar 12, 2020 at 12:23:46PM +0800, Kai-Heng Feng wrote: >> Hi, >> >>> On Mar 11, 2020, at 18:28, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: >>> >>> Hi, >>> >>> On Wed, Mar 11, 2020 at 12:52:49PM +0800, Kai-Heng Feng wrote: >>>> Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the >>>> PCIe spec") added a 1100ms delay on resume for bridges that don't >>>> support Link Active Reporting. >>>> >>>> The commit also states that the delay can be skipped for S3, as the >>>> firmware should already handled the case for us. >>> >>> Delay can be skipped if the firmware provides _DSM with function 8 >>> implemented according to PCI firmwre spec 3.2 sec 4.6.8. >> >> As someone who doesn't have access to the PCI spec... >> Questions below. >> >>> >>>> So let's skip the link training delay for S3, to save 1100ms resume >>>> time. >>>> >>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >>>> --- >>>> drivers/pci/pci-driver.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >>>> index 0454ca0e4e3f..3050375bad04 100644 >>>> --- a/drivers/pci/pci-driver.c >>>> +++ b/drivers/pci/pci-driver.c >>>> @@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev) >>>> pci_fixup_device(pci_fixup_resume_early, pci_dev); >>>> pcie_pme_root_status_cleanup(pci_dev); >>>> >>>> - if (!skip_bus_pm && prev_state == PCI_D3cold) >>>> + if (!skip_bus_pm && prev_state == PCI_D3cold >>>> + && !pm_resume_via_firmware()) >>> >>> So this would need to check for the _DSM result as well. We do evaluate >>> it in pci_acpi_optimize_delay() (drivers/pci/pci-acpi.c) and that ends >>> up lowering ->d3cold_delay so maybe check that here. >> >> Do we need to wait for d3cold_delay here? >> Or we can also skip that as long as pci_acpi_dsm_guid and FUNCTION_DELAY_DSM present? > > Actually I think pci_bridge_wait_for_secondary_bus() already takes it > into account. Have you checked if the BIOS has this _DSM implemented in > the first place? -[0000:00]-+-00.0 Intel Corporation Device 9b44 +-1c.0-[03-3b]----00.0-[04-3b]--+-00.0-[05]----00.0 Intel Corporation JHL7540 Thunderbolt 3 NHI [Titan Ridge 2C 2018] | +-01.0-[06-3a]-- | \-02.0-[3b]----00.0 Intel Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge 2C 2018] 00:1c.0 has _DSM implemented. How do I check for the Thunderbolt device? It doesn't seem to have a fixed _ADR so I don't know how to locate it in DSDT/SSDT table. Log with additional debug message: [ 948.813025] ACPI: EC: interrupt unblocked [ 948.925017] pcieport 0000:00:01.0: pcie_wait_for_link_delay sleep 1100ms [ 949.065466] pcieport 0000:04:00.0: pcie_wait_for_link_delay sleep 1100ms [ 949.065468] pcieport 0000:04:02.0: pcie_wait_for_link_delay sleep 1100ms 00:01.0 is the port for discrete graphics. Kai-Heng ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/PM: Skip link training delay for S3 resume 2020-03-12 9:45 ` Kai-Heng Feng @ 2020-03-12 10:08 ` Mika Westerberg 0 siblings, 0 replies; 6+ messages in thread From: Mika Westerberg @ 2020-03-12 10:08 UTC (permalink / raw) To: Kai-Heng Feng; +Cc: bhelgaas, linux-pci, linux-kernel On Thu, Mar 12, 2020 at 05:45:32PM +0800, Kai-Heng Feng wrote: > > > > On Mar 12, 2020, at 16:04, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > > > On Thu, Mar 12, 2020 at 12:23:46PM +0800, Kai-Heng Feng wrote: > >> Hi, > >> > >>> On Mar 11, 2020, at 18:28, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > >>> > >>> Hi, > >>> > >>> On Wed, Mar 11, 2020 at 12:52:49PM +0800, Kai-Heng Feng wrote: > >>>> Commit ad9001f2f411 ("PCI/PM: Add missing link delays required by the > >>>> PCIe spec") added a 1100ms delay on resume for bridges that don't > >>>> support Link Active Reporting. > >>>> > >>>> The commit also states that the delay can be skipped for S3, as the > >>>> firmware should already handled the case for us. > >>> > >>> Delay can be skipped if the firmware provides _DSM with function 8 > >>> implemented according to PCI firmwre spec 3.2 sec 4.6.8. > >> > >> As someone who doesn't have access to the PCI spec... > >> Questions below. > >> > >>> > >>>> So let's skip the link training delay for S3, to save 1100ms resume > >>>> time. > >>>> > >>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > >>>> --- > >>>> drivers/pci/pci-driver.c | 3 ++- > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > >>>> index 0454ca0e4e3f..3050375bad04 100644 > >>>> --- a/drivers/pci/pci-driver.c > >>>> +++ b/drivers/pci/pci-driver.c > >>>> @@ -916,7 +916,8 @@ static int pci_pm_resume_noirq(struct device *dev) > >>>> pci_fixup_device(pci_fixup_resume_early, pci_dev); > >>>> pcie_pme_root_status_cleanup(pci_dev); > >>>> > >>>> - if (!skip_bus_pm && prev_state == PCI_D3cold) > >>>> + if (!skip_bus_pm && prev_state == PCI_D3cold > >>>> + && !pm_resume_via_firmware()) > >>> > >>> So this would need to check for the _DSM result as well. We do evaluate > >>> it in pci_acpi_optimize_delay() (drivers/pci/pci-acpi.c) and that ends > >>> up lowering ->d3cold_delay so maybe check that here. > >> > >> Do we need to wait for d3cold_delay here? > >> Or we can also skip that as long as pci_acpi_dsm_guid and FUNCTION_DELAY_DSM present? > > > > Actually I think pci_bridge_wait_for_secondary_bus() already takes it > > into account. Have you checked if the BIOS has this _DSM implemented in > > the first place? > > -[0000:00]-+-00.0 Intel Corporation Device 9b44 > +-1c.0-[03-3b]----00.0-[04-3b]--+-00.0-[05]----00.0 Intel Corporation JHL7540 Thunderbolt 3 NHI [Titan Ridge 2C 2018] > | +-01.0-[06-3a]-- > | \-02.0-[3b]----00.0 Intel Corporation JHL7540 Thunderbolt 3 USB Controller [Titan Ridge 2C 2018] > > 00:1c.0 has _DSM implemented. > How do I check for the Thunderbolt device? Most likely you see it only under the root port (1c.0) so check /sys/bus/pci/devices/0000:00:1c.0/firmware_node/path and match that one to the ACPI tables. > It doesn't seem to have a fixed _ADR so I don't know how to locate it in DSDT/SSDT table. > > Log with additional debug message: > [ 948.813025] ACPI: EC: interrupt unblocked > [ 948.925017] pcieport 0000:00:01.0: pcie_wait_for_link_delay sleep 1100ms > [ 949.065466] pcieport 0000:04:00.0: pcie_wait_for_link_delay sleep 1100ms > [ 949.065468] pcieport 0000:04:02.0: pcie_wait_for_link_delay sleep 1100ms > > 00:01.0 is the port for discrete graphics. There is something wrong somewhere because the 1100ms sleep is totally not expected to happen, but I think this is the same issue that we discuss on another thread so let's use that thread instead to avoid confusion :) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-12 10:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-11 4:52 [PATCH] PCI/PM: Skip link training delay for S3 resume Kai-Heng Feng 2020-03-11 10:28 ` Mika Westerberg 2020-03-12 4:23 ` Kai-Heng Feng 2020-03-12 8:04 ` Mika Westerberg 2020-03-12 9:45 ` Kai-Heng Feng 2020-03-12 10:08 ` Mika Westerberg
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).