* [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).