2016-12-09 22:44:28

by Vaibhav Shankar

[permalink] [raw]
Subject: [PATCH] PCI: pciehp: Optimize PCIe root resume time

On Apollolake platforms, PCIe rootport takes a long time to resume
from S3. With 100ms delay before read pci conf, rootport takes
~200ms during resume.

commit 2f5d8e4ff947 ("PCI: pciehp: replace unconditional sleep with
config space access check") is the one that added the 100ms delay
before reading pci conf.

This patch removes the 100ms delay.By removing the delay, the
PCIe root port takes ~16ms during resume. As per PCIe spec, we
only require 1000ms delay. This delay is provide by
pci_bus_check_dev() function.

With 100ms delay:
[ 155.102713] calling 0000:00:14.0+ @ 70, parent: pci0000:00, cb: pci_pm_resume_noirq
[ 155.119337] call 0000:00:14.0+ returned 0 after 16231 usecs
[ 155.119467] calling 0000:01:00.0+ @ 5845, parent: 0000:00:14.0, cb: pci_pm_resume_noirq
[ 155.321670] call 0000:00:14.0+ returned 0 after 185327 usecs
[ 155.321743] calling 0000:01:00.0+ @ 5849, parent: 0000:00:14.0, cb: pci_pm_resume

After removing 100ms delay:
[ 36.624709] calling 0000:00:14.0+ @ 4434, parent: pci0000:00, cb: pci_pm_resume_noirq
[ 36.641367] call 0000:00:14.0+ returned 0 after 16263 usecs
[ 36.652458] calling 0000:00:14.0+ @ 4443, parent: pci0000:00, cb: pci_pm_resume
[ 36.652673] call 0000:00:14.0+ returned 0 after 208 usecs
[ 36.652863] calling 0000:01:00.0+ @ 4442, parent: 0000:00:14.0, cb: pci_pm_resume

Signed-off-by: Vaibhav Shankar <[email protected]>
---
drivers/pci/hotplug/pciehp_hpc.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 5c24e93..08357e7 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -311,8 +311,6 @@ int pciehp_check_link_status(struct controller *ctrl)
else
msleep(1000);

- /* wait 100ms before read pci conf, and try in 1s */
- msleep(100);
found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
PCI_DEVFN(0, 0));

--
1.7.9.5


2016-12-10 00:27:50

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] PCI: pciehp: Optimize PCIe root resume time

[+cc Yinghai, author of 2f5d8e4ff947]

On Fri, Dec 09, 2016 at 02:43:26PM -0800, Vaibhav Shankar wrote:
> On Apollolake platforms, PCIe rootport takes a long time to resume
> from S3. With 100ms delay before read pci conf, rootport takes
> ~200ms during resume.
>
> commit 2f5d8e4ff947 ("PCI: pciehp: replace unconditional sleep with
> config space access check") is the one that added the 100ms delay
> before reading pci conf.
>
> This patch removes the 100ms delay.By removing the delay, the
> PCIe root port takes ~16ms during resume. As per PCIe spec, we
> only require 1000ms delay. This delay is provide by
> pci_bus_check_dev() function.
>
> With 100ms delay:
> [ 155.102713] calling 0000:00:14.0+ @ 70, parent: pci0000:00, cb: pci_pm_resume_noirq
> [ 155.119337] call 0000:00:14.0+ returned 0 after 16231 usecs
> [ 155.119467] calling 0000:01:00.0+ @ 5845, parent: 0000:00:14.0, cb: pci_pm_resume_noirq
> [ 155.321670] call 0000:00:14.0+ returned 0 after 185327 usecs
> [ 155.321743] calling 0000:01:00.0+ @ 5849, parent: 0000:00:14.0, cb: pci_pm_resume
>
> After removing 100ms delay:
> [ 36.624709] calling 0000:00:14.0+ @ 4434, parent: pci0000:00, cb: pci_pm_resume_noirq
> [ 36.641367] call 0000:00:14.0+ returned 0 after 16263 usecs
> [ 36.652458] calling 0000:00:14.0+ @ 4443, parent: pci0000:00, cb: pci_pm_resume
> [ 36.652673] call 0000:00:14.0+ returned 0 after 208 usecs
> [ 36.652863] calling 0000:01:00.0+ @ 4442, parent: 0000:00:14.0, cb: pci_pm_resume
>
> Signed-off-by: Vaibhav Shankar <[email protected]>
> ---
> drivers/pci/hotplug/pciehp_hpc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 5c24e93..08357e7 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -311,8 +311,6 @@ int pciehp_check_link_status(struct controller *ctrl)
> else
> msleep(1000);
>
> - /* wait 100ms before read pci conf, and try in 1s */
> - msleep(100);
> found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
> PCI_DEVFN(0, 0));
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2016-12-11 01:19:14

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] PCI: pciehp: Optimize PCIe root resume time

On Fri, Dec 09, 2016 at 06:27:44PM -0600, Bjorn Helgaas wrote:
> [+cc Yinghai, author of 2f5d8e4ff947]
>
> On Fri, Dec 09, 2016 at 02:43:26PM -0800, Vaibhav Shankar wrote:
> > On Apollolake platforms, PCIe rootport takes a long time to resume
> > from S3. With 100ms delay before read pci conf, rootport takes
> > ~200ms during resume.
> >
> > commit 2f5d8e4ff947 ("PCI: pciehp: replace unconditional sleep with
> > config space access check") is the one that added the 100ms delay
> > before reading pci conf.
> >
> > This patch removes the 100ms delay.By removing the delay, the
> > PCIe root port takes ~16ms during resume. As per PCIe spec, we
> > only require 1000ms delay. This delay is provide by
> > pci_bus_check_dev() function.
> >
> > Signed-off-by: Vaibhav Shankar <[email protected]>
> > ---
> > drivers/pci/hotplug/pciehp_hpc.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> > index 5c24e93..08357e7 100644
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -311,8 +311,6 @@ int pciehp_check_link_status(struct controller *ctrl)
> > else
> > msleep(1000);
> >
> > - /* wait 100ms before read pci conf, and try in 1s */
> > - msleep(100);
> > found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
> > PCI_DEVFN(0, 0));

that msleep(100) is from that commit:

- /*
- * If the port supports Link speeds greater than 5.0 GT/s, we
- * must wait for 100 ms after Link training completes before
- * sending configuration request.
- */
- if (ctrl->pcie->port->subordinate->max_bus_speed > PCIE_SPEED_5_0GT)
- msleep(100);

so we should put the checking back.

diff --git a/drivers/pci/hotplug/pciehp_hpc.c
b/drivers/pci/hotplug/pciehp_hpc.c
index 026830a..1b0fc0b 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -311,8 +311,15 @@ int pciehp_check_link_status(struct controller *ctrl)
else
msleep(1000);

- /* wait 100ms before read pci conf, and try in 1s */
- msleep(100);
+ /*
+ * If the port supports Link speeds greater than 5.0 GT/s, we
+ * must wait for 100 ms after Link training completes before
+ * sending configuration request.
+ */
+ if (ctrl->pcie->port->subordinate->max_bus_speed > PCIE_SPEED_5_0GT)
+ msleep(100);
+
+ /* try in 1s */
found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
PCI_DEVFN(0, 0));



Thanks

Yinghai

2016-12-13 00:32:52

by Vaibhav Shankar

[permalink] [raw]
Subject: [PATCH v2] PCI: pciehp: Optimize PCIe root resume time

On Apollolake platforms, PCIe rootport takes a long time to resume
from S3. With 100ms delay before read pci conf, rootport takes
~200ms during resume.

commit 2f5d8e4ff947 ("PCI: pciehp: replace unconditional sleep with
config space access check") is the one that added the 100ms delay
before reading pci conf.

This patch includes a condition check for 100ms dealy before reading
PCIe conf. This delay in included only when PCIe
max_bus_speed > 5.0 GT/s. Root port takes ~16ms during resume.

With 100ms delay:
[ 155.102713] calling 0000:00:14.0+ @ 70, parent: pci0000:00, cb: pci_pm_resume_noirq
[ 155.119337] call 0000:00:14.0+ returned 0 after 16231 usecs
[ 155.119467] calling 0000:01:00.0+ @ 5845, parent: 0000:00:14.0, cb: pci_pm_resume_noirq
[ 155.321670] call 0000:00:14.0+ returned 0 after 185327 usecs
[ 155.321743] calling 0000:01:00.0+ @ 5849, parent: 0000:00:14.0, cb: pci_pm_resume

With Condition check:
[ 36.624709] calling 0000:00:14.0+ @ 4434, parent: pci0000:00, cb: pci_pm_resume_noirq
[ 36.641367] call 0000:00:14.0+ returned 0 after 16263 usecs
[ 36.652458] calling 0000:00:14.0+ @ 4443, parent: pci0000:00, cb: pci_pm_resume
[ 36.652673] call 0000:00:14.0+ returned 0 after 208 usecs
[ 36.652863] calling 0000:01:00.0+ @ 4442, parent: 0000:00:14.0, cb: pci_pm_resume

Signed-off-by: Vaibhav Shankar <[email protected]>
---
changes in v2:
- Modify patch description.
- Add condition check for 100ms delay before read pci conf as
suggested by Yinghai.

drivers/pci/hotplug/pciehp_hpc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index b57fc6d..2b10e5f 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -311,8 +311,15 @@ int pciehp_check_link_status(struct controller *ctrl)
else
msleep(1000);

- /* wait 100ms before read pci conf, and try in 1s */
- msleep(100);
+ /*
+ * If the port supports Link speeds greater than 5.0 GT/s, we
+ * must wait for 100 ms after Link training completes before
+ * sending configuration request.
+ */
+ if (ctrl->pcie->port->subordinate->max_bus_speed > PCIE_SPEED_5_0GT)
+ msleep(100);
+
+ /* try in 1s */
found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
PCI_DEVFN(0, 0));

--
1.7.9.5