mbox series

[v2,0/2] PCI: Add missing link delays

Message ID 20191004123947.11087-1-mika.westerberg@linux.intel.com (mailing list archive)
Headers show
Series PCI: Add missing link delays | expand

Message

Mika Westerberg Oct. 4, 2019, 12:39 p.m. UTC
Hi,

This is second version of the reworked PCIe link delay patch posted earlier
here:

  https://patchwork.kernel.org/patch/11106611/

Changes from v1:

  * Introduce pcie_wait_for_link_delay() in a separate patch
  * Tidy up changelog, remove some debug output
  * Rename pcie_wait_downstream_accessible() to
    pci_bridge_wait_for_secondary_bus() and make it generic to all PCI
    bridges.
  * Handle Tpvrh + Trhfa for conventional PCI even though we don't do PM
    for them right now.
  * Use pci_dbg() instead of dev_dbg().
  * Dropped check for pm_suspend_no_platform() and only check for D3cold.
  * Drop pcie_get_downstream_delay(), same delay applies equally to all
    devices (it is not entirely clear from the spec).

I'm still checking for downstream device because I think we can skip the
delays if there is nothing connected. The reason is that if device is added
when the downstream/root port is in D3 the delay is handled by pciehp in
its board_added(). In case of ACPI hotplug the firmware is supposed to
configure the device (and handle the delay).

I also checked we do resume sibling devices in paraller (I think due to
async_suspend).

@Matthias, @Paul and @Nicholas, I appreciate if you could check that this
does not cause any issues for your systems.

Mika Westerberg (2):
  PCI: Introduce pcie_wait_for_link_delay()
  PCI: Add missing link delays required by the PCIe spec

 drivers/pci/pci-driver.c |  18 +++++++
 drivers/pci/pci.c        | 113 +++++++++++++++++++++++++++++++++++----
 drivers/pci/pci.h        |   1 +
 3 files changed, 122 insertions(+), 10 deletions(-)

Comments

Matthias Andree Oct. 4, 2019, 12:57 p.m. UTC | #1
Am 04.10.19 um 14:39 schrieb Mika Westerberg:
> @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
> does not cause any issues for your systems.

Just to be sure: is this intended to be applied against the 5.4-rc*
master branch?
Mika Westerberg Oct. 4, 2019, 1:06 p.m. UTC | #2
On Fri, Oct 04, 2019 at 02:57:21PM +0200, Matthias Andree wrote:
> Am 04.10.19 um 14:39 schrieb Mika Westerberg:
> > @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
> > does not cause any issues for your systems.
> 
> Just to be sure: is this intended to be applied against the 5.4-rc*
> master branch?

Yes, it applies on top of v5.4-rc1.
Matthias Andree Oct. 5, 2019, 7:34 a.m. UTC | #3
Am 04.10.19 um 15:06 schrieb Mika Westerberg:
> On Fri, Oct 04, 2019 at 02:57:21PM +0200, Matthias Andree wrote:
>> Am 04.10.19 um 14:39 schrieb Mika Westerberg:
>>> @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
>>> does not cause any issues for your systems.
>> Just to be sure: is this intended to be applied against the 5.4-rc*
>> master branch?
> Yes, it applies on top of v5.4-rc1.

I am sorry to say that I cannot currently test - my computer has a
GeForce 1060-6GB an no onboard/on-chip graphics.
The nvidia module 435.21 does not compile against 5.4-rc* for me (5.3.1
was fine).

For some reasons I don't understand, it first complains about missing or
empty  Module.symvers, (which I do have and which has 12967 lines)
and if I bypass that check, it complains about undeclared DRIVER_PRIME
"here (outside a function)" - sorry for the German locale:

/var/lib/dkms/nvidia/435.21/build/nvidia-drm/nvidia-drm-drv.c:662:44:
Fehler: »DRIVER_PRIME« ist hier (außerhalb einer Funktion) nicht
deklariert; meinten Sie »DRIVER_PCI_DMA«?
  662 |     .driver_features        = DRIVER_GEM | DRIVER_PRIME |
DRIVER_RENDER,
      |                                            ^~~~~~~~~~~~
      |                                            DRIVER_PCI_DMA

I need NOT try this hardware without nvidia proprietary driver, nouveau
has always been underfeatured and I never got suspend/resume working
with it, so I don't bother else it would skew the findings.

(Someone let me know if switching to AMD 5x00 (XT) is worthwhile or
premature. Vega and earlier consume way too much power to bother. I'm
not buying a new PSU.)
Mika Westerberg Oct. 7, 2019, 9:32 a.m. UTC | #4
On Sat, Oct 05, 2019 at 09:34:41AM +0200, Matthias Andree wrote:
> Am 04.10.19 um 15:06 schrieb Mika Westerberg:
> > On Fri, Oct 04, 2019 at 02:57:21PM +0200, Matthias Andree wrote:
> >> Am 04.10.19 um 14:39 schrieb Mika Westerberg:
> >>> @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
> >>> does not cause any issues for your systems.
> >> Just to be sure: is this intended to be applied against the 5.4-rc*
> >> master branch?
> > Yes, it applies on top of v5.4-rc1.
> 
> I am sorry to say that I cannot currently test - my computer has a
> GeForce 1060-6GB an no onboard/on-chip graphics.
> The nvidia module 435.21 does not compile against 5.4-rc* for me (5.3.1
> was fine).

I think the two patches should apply cleanly on 5.3.x as well.

> For some reasons I don't understand, it first complains about missing or
> empty  Module.symvers, (which I do have and which has 12967 lines)
> and if I bypass that check, it complains about undeclared DRIVER_PRIME
> "here (outside a function)" - sorry for the German locale:

Possibly v5.4-rcX moved/renamed some symbol(s) which than makes the
out-of-tree driver fail to build.
Matthias Andree Oct. 7, 2019, 3:15 p.m. UTC | #5
Am 07.10.19 um 11:32 schrieb Mika Westerberg:
> On Sat, Oct 05, 2019 at 09:34:41AM +0200, Matthias Andree wrote:
>> Am 04.10.19 um 15:06 schrieb Mika Westerberg:
>>> On Fri, Oct 04, 2019 at 02:57:21PM +0200, Matthias Andree wrote:
>>>> Am 04.10.19 um 14:39 schrieb Mika Westerberg:
>>>>> @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
>>>>> does not cause any issues for your systems.
>>>> Just to be sure: is this intended to be applied against the 5.4-rc*
>>>> master branch?
>>> Yes, it applies on top of v5.4-rc1.
>> I am sorry to say that I cannot currently test - my computer has a
>> GeForce 1060-6GB an no onboard/on-chip graphics.
>> The nvidia module 435.21 does not compile against 5.4-rc* for me (5.3.1
>> was fine).
> I think the two patches should apply cleanly on 5.3.x as well.

Mika, that worked.

With your two patches on top of Linux 5.3.4, two Suspend-to-RAM cycles
(ACPI S3), one Suspend-to-disk cycle (ACPI S4),
no regressions observed => success?

Let me know off-list if you need any of my "usual logs" from my test script.

One blank line added to delineate Greg's release from your patches:

> * 9c2dfb396722 2019-10-04 | PCI: Add missing link delays required by
> the PCIe spec (HEAD -> linux-5.3.y) [Mika Westerberg]
> * 00103c8c3fa8 2019-10-04 | PCI: Introduce pcie_wait_for_link_delay()
> [Mika Westerberg]
>
> * ed56826f1779 2019-10-05 | Linux 5.3.4 (tag: v5.3.4,
> stable/linux-5.3.y) [Greg Kroah-Hartman]
> * d0b85a37c06b 2019-09-04 | platform/chrome: cros_ec_rpmsg: Fix race
> with host command when probe failed [Pi-Hsun Shih]
> * bec8c6dec605 2019-09-22 | mt76: mt7615: fix mt7615 firmware path
> definitions [Lorenzo Bianconi]
> * 5dab55b417ca 2019-07-02 | mt76: mt7615: always release sem in
> mt7615_load_patch [Lorenzo Bianconi]
> * 88688a6cd741 2019-09-09 | md/raid0: avoid RAID0 data corruption due
> to layout confusion. [NeilBrown]
Regards,
Matthias
Mika Westerberg Oct. 8, 2019, 9:05 a.m. UTC | #6
On Mon, Oct 07, 2019 at 05:15:24PM +0200, Matthias Andree wrote:
> Am 07.10.19 um 11:32 schrieb Mika Westerberg:
> > On Sat, Oct 05, 2019 at 09:34:41AM +0200, Matthias Andree wrote:
> >> Am 04.10.19 um 15:06 schrieb Mika Westerberg:
> >>> On Fri, Oct 04, 2019 at 02:57:21PM +0200, Matthias Andree wrote:
> >>>> Am 04.10.19 um 14:39 schrieb Mika Westerberg:
> >>>>> @Matthias, @Paul and @Nicholas, I appreciate if you could check that this
> >>>>> does not cause any issues for your systems.
> >>>> Just to be sure: is this intended to be applied against the 5.4-rc*
> >>>> master branch?
> >>> Yes, it applies on top of v5.4-rc1.
> >> I am sorry to say that I cannot currently test - my computer has a
> >> GeForce 1060-6GB an no onboard/on-chip graphics.
> >> The nvidia module 435.21 does not compile against 5.4-rc* for me (5.3.1
> >> was fine).
> > I think the two patches should apply cleanly on 5.3.x as well.
> 
> Mika, that worked.
> 
> With your two patches on top of Linux 5.3.4, two Suspend-to-RAM cycles
> (ACPI S3), one Suspend-to-disk cycle (ACPI S4),
> no regressions observed => success?

Yes, if it did not hang during resume (because of the PME loop) I think
it should be declared as success :)

Thanks a lot for testing!