On Tue, May 31, 2016 at 01:07:57PM +0200, Lukas Wunner wrote: > On Tue, May 31, 2016 at 01:47:14PM +0300, Mika Westerberg wrote: > > On Tue, May 31, 2016 at 12:40:51PM +0200, Lukas Wunner wrote: > > > On Tue, May 31, 2016 at 11:58:05AM +0300, Mika Westerberg wrote: > > > > To summarize the next steps. I will send new version of the > > > > PCI PM patches with following changes. > > > > > > > > - Drop this 50ms patch, we should have the PCIe 100ms delay already > > > > covered. > > > > > > > > - Increase runtime PM autosuspend time from 10ms to 500ms (or whatever > > > > is the prefered default). > > > > > > I did some tests, turns out the autosuspend delay need not be increased > > > to prevent the Thunderbolt hotplug ports from suspending between > > > "enabling device" and loading the pciehp driver, however the following > > > is needed: > > > > > > diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c > > > index 7860ab3..1d1fb1c 100644 > > > --- a/drivers/pci/pcie/portdrv_pci.c > > > +++ b/drivers/pci/pcie/portdrv_pci.c > > > @@ -238,6 +238,7 @@ static int pcie_portdrv_probe(struct pci_dev *dev, > > > pm_runtime_set_autosuspend_delay(&dev->dev, 10); > > > pm_runtime_use_autosuspend(&dev->dev); > > > pm_runtime_put_autosuspend(&dev->dev); > > > + pm_runtime_mark_last_busy(&dev->dev); > > > pm_runtime_allow(&dev->dev); > > > } > > > > I still prefer increasing the autosuspend delay. The above looks hackish > > and does not work if it takes more than 10ms to get to the tbt driver > > probe. > > > > Did you try if it also works with 500ms delay? > > I tried 150 ms and it didn't work. The pm_runtime_mark_last_busy() > is needed no matter how much you increase the autosuspend delay. > This isn't hackish. :-) The issue is that pm_runtime_mark_last_busy() > has never been called so far, so dev->power.last_busy == 0. Yes, it looks like it is really needed. I somehow remembered that pm_runtime_set_autosuspend_delay() sets that automatically. So I take back my hackish comment ;-) > The PM core thinks that the device can suspend right away because it's > last been busy more than 2 seconds ago. Right. > One could argue though if pm_runtime_mark_last_busy() should come > before pm_runtime_put_autosuspend(). Usually it should, but in this > case it doesn't matter because pm_runtime_allow() hasn't been called > yet and the PCI core initializes devices to pm_runtime_forbid(). I'm going to change the code to look like following (pm_runtime_mark_last_busy() gets called before pm_runtime_put_autosuspend() even if not strictly needed): pm_runtime_set_autosuspend_delay(&dev->dev, 100); pm_runtime_use_autosuspend(&dev->dev); pm_runtime_mark_last_busy(&dev->dev); pm_runtime_put_autosuspend(&dev->dev); pm_runtime_allow(&dev->dev); Note I'm still increasing default autosuspend delay from 10ms to 100ms. Does the above work for you? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
