On Wed, May 1, 2013 at 2:31 AM, Emmanuel Grumbach <egrumbach@xxxxxxxxx> wrote: > [from Bjorn's mail] >> In Emmanuel's case, we don't get _OSC control, so >> pci_disable_link_state() does nothing. > > Right, but this is true with the specific log I sent to you. Is it > possible that another platform / BIOS, we *will* get _OSC control and > that pci_disable_link_state() will actually do something? In that case > I would prefer not to remove the call to pcie_disable_link_state(). Yes, absolutely, on many platforms we will get _OSC control, and pci_disable_link_state() will work as expected. The problem is that the driver doesn't have a good way to know whether pci_disable_link() did anything or not. Today I think we have: 1) If the BIOS grants the OS permission to control PCIe services via _OSC, pci_disable_link_state() works and L1 will be disabled. 2) If the BIOS does not grant permission, pci_disable_link_state() does nothing and L1 may be enabled or not depending on what configuration the BIOS did. If the device really doesn't work reliably when L1 is enabled, we're currently at the mercy of the BIOS -- if the BIOS enables L1 but doesn't grant us permission via _OSC, L1 will remain enabled (as it is on your system). > Also - in the log I sent you, we don't get _OSC control, but I can see > 'disabling PCIe ASPM" ... and it *is* enabled (since L1 works). So I > am kinda scratching my head here... The code is a bit convoluted, but I *think* the "disabling PCIe ASPM" message basically means "Linux is not going to touch ASPM configuration," so we just use whatever the BIOS set up. >> The general issue here is that Windows will (as far as we've been able >> to determine) never touch ASPM registers unless given PCIe control via >> _OSC. Drivers are presumably able to override this by hitting >> configuration registers themselves. The pci_disable_link_state() >> functions are currently broadly equivalent to the helper functions >> provided in the Windows .inf files, and if drivers really want to >> disable the control themselves then they can do so. The Windows mechanism (described in "PCI Express in Depth for Windows Vista and Beyond" [1]) is to put a "Needs=PciASPMOptOut" statement in the .inf file. My impression is that if the BIOS enables ASPM but doesn't grant control to the OS, Vista will generally leave the ASPM config alone. I guess the question is what happens when a driver .inf specifies PciASPMOptOut in this case. I don't think it makes sense to provide this mechanism to driver writers, but make it only work if the BIOS grants control. The doc doesn't mention anything about drivers needing to do anything extra to cover the case when the BIOS didn't grant control. So my guess is that when PciASPMOptOut is specified, Vista will turn off ASPM for that device in either case. But maybe you've tested this and determined otherwise? >> The only time this should be relevant is if (a) the BIOS has enabled L1 >> on iwlwifi, (b) the BIOS has disabled ASPM control, and (c) the hardware >> doesn't work with L1 enabled. Are there really cases where that's true? > > After having read (a bit) about _OSC I can say the following: > L1 is a must for the Windows driver - so that it doesn't sounds > reasonable that the .inf of the Windows driver for Intel Wireless NICs > disables ASPM through _OSC or through any other mechanism. > (a) - this is really platform / BIOS dependent, and I can't test all > the platform out there > (b) - ditto > (c) - the HW *should* work with L1 enabled but we may very well have a > bug in the driver. I have tried to check the way we initialize the HW > and it seems fine, but one never knows. Do you know if the Windows driver for these iwlwifi devices specifies PciASPMOptOut? If it doesn't, that's a pretty good clue that the Linux driver shouldn't need to use pci_disable_link_state(). Bjorn [1] http://download.microsoft.com/download/5/b/9/5b97017b-e28a-4bae-ba48-174cf47d23cd/cpa070_wh06.ppt -- 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
