Hi Sinan, Bjorn, > >Hi Sinan > >>Hi Mayurkumar, >> >>On 3/30/2017 9:30 AM, Sinan Kaya wrote: >>> When the operating system is booted with the default ASPM policy >>> (POLICY_DEFAULT), current code is querying the enable/disable >>> states from ASPM registers to determine the policy. >>> >>> For example, a BIOS could set the power saving state to performance >>> and clear all ASPM control registers. A balanced ASPM policy could >>> enable L0s and disable L1. A power conscious BIOS could enable both >>> L0s and L1 to trade off latency and performance vs. power. >>> >>> After hotplug removal, pcie_aspm_exit_link_state() function clears >>> the ASPM registers. An insertion following hotplug removal reads >>> incorrect policy as ASPM disabled even though ASPM was enabled >>> during boot. >>> >>> This is caused by the fact that same function is used for reconfiguring >>> ASPM regardless of the power on state. >>> >>> ------------------------ >>> Changes from v6 (https://www.spinics.net/lists/arm-kernel/msg572876.html) >>> ------------------------ >>> - revert the accidental parent check in bridge remove >>> >>> Sinan Kaya (5): >>> PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities() >>> PCI/ASPM: split pci_aspm_init() into two >>> PCI/ASPM: add init hook to device_add >>> PCI/ASPM: save power on values during bridge init >>> PCI/ASPM: move link_state cleanup to bridge remove >>> >>> drivers/pci/pcie/aspm.c | 137 ++++++++++++++++++++++++++++++++---------------- >>> drivers/pci/probe.c | 3 ++ >>> drivers/pci/remove.c | 3 +- >>> include/linux/pci.h | 2 + >>> 4 files changed, 98 insertions(+), 47 deletions(-) >>> >> >>Did you get a chance to test? >> > >Not yet. Will get back in this week sometimes to give you some feedback. > The patch seems to be working for ASPM L1 so far. I am seeing a problem now with your patches for L1.2 as following: ASPM L1.2 does not get enabled on the Upstream as well as downstream port due to following reasons. In the case, I see now, if the EP is not connected while reboot of the Machine, Root Port does not configure It's own L1.2 also from the BIOS. (I am not sure whether it's even advisable to enable L1.2 on root Port when no downstream device is not connected to it as it may have an impact on CLKREQ# and device is connected At later stage may have a problem with it) Later when the Device get connected, BIOS configures L1.2 for Root port and EP but due to policy set to incorrect, Kernel disables ASPM L1.2. Actually, The enabling/configuring of ASPM L1.2 is opposite then ASPM L1. In case of L1, Upstream must configure first L1 and then downstream according to PCIe spec. In case of L1.2, Downstream must configure L1.2 and then upstream according (to L1.2 ECN spec. -> 5.5.4. L1 PM substates Configuration). @Bjorn: I even found that programming of pcie_config_aspm_l1ss() sub-states is done in the opposite way than described in the spec., The spec. says following and correct me If I am wrong or I misinterpret the spec. : 5.5.4. L1 PM Substates Configuration The Setting of any enable bit must be performed at the Downstream Port before the corresponding bit is permitted to be Set at the Upstream Port. If any L1 PM Substates enable bit is at a later time to be cleared, the enable bit(s) must be cleared in the Upstream Port before the corresponding enable bit(s) are permitted to be cleared in the Downstream Port. Thanks Mayur > > >Thanks >Mayur > >>Sinan >> >>-- >>Sinan Kaya >>Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. >>Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. >Intel Deutschland GmbH >Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany >Tel: +49 89 99 8853-0, www.intel.de >Managing Directors: Christin Eisenschmid, Christian Lamprechter >Chairperson of the Supervisory Board: Nicole Lau >Registered Office: Munich >Commercial Register: Amtsgericht Muenchen HRB 186928 Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928
