On Thu, May 26, 2016 at 01:25:20PM +0300, Mika Westerberg wrote:

On Thu, May 26, 2016 at 12:10:06PM +0200, Lukas Wunner wrote:

quoted

On Wed, May 25, 2016 at 06:04:56PM +0300, Mika Westerberg wrote:

quoted

The PCI PM 1.2 specification requires minimum of 50ms before function on a
bus is accessed after the bus is transitioned from B2 to B0. Now that we
actually power down bridges we should make sure the specification is
followed accordingly.

This patch has the unfortunate side effect of increasing boot time on
Macs with Thunderbolt by 320 ms. Granted, it's not much, but still
noticeable:

Yes, that's the drawback.

quoted

[    2.358041] pcieport 0000:06:03.0: enabling device (0000 -> 0002)
[    2.358195] pcieport 0000:06:03.0: rpm_idle
[    2.358222] pcieport 0000:06:03.0: rpm_suspend
[    2.411664] pcieport 0000:06:04.0: enabling device (0000 -> 0003)
[    2.411821] pcieport 0000:06:04.0: rpm_idle
[    2.411848] pcieport 0000:06:04.0: rpm_suspend
[    2.467664] pcieport 0000:06:05.0: enabling device (0000 -> 0003)
[    2.467817] pcieport 0000:06:05.0: rpm_idle
[    2.467843] pcieport 0000:06:05.0: rpm_suspend
[    2.523664] pcieport 0000:06:06.0: enabling device (0000 -> 0002)
[    2.523822] pcieport 0000:06:06.0: rpm_idle
[    2.523848] pcieport 0000:06:06.0: rpm_suspend
[    2.579685] pci_hotplug: PCI Hot Plug PCI Core version: 0.5
[    2.579722] pcieport 0000:06:03.0: rpm_resume
[    2.635750] pciehp 0000:06:03.0:pcie204: Slot #3 AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Interlock- NoCompl- LLActRep+
[    2.635853] pciehp 0000:06:03.0:pcie204: service driver pciehp loaded
[    2.635858] pcieport 0000:06:03.0: rpm_idle
[    2.635886] pcieport 0000:06:04.0: rpm_resume
[    2.647645] pcieport 0000:06:03.0: rpm_suspend
[    2.691747] pciehp 0000:06:04.0:pcie204: Slot #4 AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Interlock- NoCompl- LLActRep+
[    2.691856] pciehp 0000:06:04.0:pcie204: service driver pciehp loaded
[    2.691859] pcieport 0000:06:04.0: rpm_idle
[    2.691888] pcieport 0000:06:05.0: rpm_resume
[    2.703649] pcieport 0000:06:04.0: rpm_suspend
[    2.747748] pciehp 0000:06:05.0:pcie204: Slot #5 AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Interlock- NoCompl- LLActRep+
[    2.747842] pciehp 0000:06:05.0:pcie204: service driver pciehp loaded
[    2.747845] pcieport 0000:06:06.0: rpm_resume
[    2.749213] pcieport 0000:06:05.0: rpm_idle
[    2.759650] pcieport 0000:06:05.0: rpm_suspend
[    2.805049] pciehp 0000:06:06.0:pcie204: Slot #6 AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+ Interlock- NoCompl- LLActRep+
[    2.806464] pciehp 0000:06:06.0:pcie204: service driver pciehp loaded
[    2.806471] pciehp: PCI Express Hot Plug Controller Driver version: 0.4
[    2.807876] intel_idle: MWAIT substates: 0x21120
[    2.807878] intel_idle: v0.4.1 model 0x3A
[    2.808170] intel_idle: lapic_timer_reliable_states 0xffffffff
[    2.808201] GHES: HEST is not enabled!
[    2.809613] pcieport 0000:06:06.0: rpm_idle
[    2.809644] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    2.810096] Linux agpgart interface v0.103
[    2.810158] AMD IOMMUv2 driver by Joerg Roedel [off-list ref]
[    2.810158] AMD IOMMUv2 functionality not available on this system
[    2.816468] pcieport 0000:06:06.0: rpm_suspend

I've added debug messages whenever ->runtime_idle / _suspend / _resume
is called for a device.

As can be seen, the PCI core now waits 50 ms after ->runtime_suspend.
Also, the ports are resumed when the pciehp service driver is loaded,
adding another 50 ms delay. For four hotplug ports, that's a total of
400 ms (versus 80 ms without the patch).

I'm wondering if the delay after ->runtime_suspend is really needed. Is
this mandated by the spec? We could perhaps increase the autosuspend_delay
in pcie_portdrv_probe() slightly to prevent the port from going to sleep
between pci_enable_device() and loading the pciehp service driver.

The PCI PM specification version 1.2 says this in chapter 4.2 (page 37):

  There is a minimum time requirement of 50 ms which must be provided by
  system software between when the bus is switched from B2 to B0 and
  when a function on the bus is accessed to allow time for the clock to
  start up and the bus to settle.

But why do we wait 50 ms when *suspending*, i.e. going from B0 to B2?

(Assuming B2 is the state when the bridge goes to D3hot, which I'm not
sure of. The spec says that the bus state may optionally be B3 if the
bridge is in D3hot.)

Not sure how much of that still applies to modern hardware.

Could you ask hardware engineers at Intel what the bus state is on
modern chipsets (say, ILK or newer) and Thunderbolt ports to clarify
this?

Thanks,

Lukas

I don't see why we cannot increase autosuspend delay time, though.