From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sinan Kaya Subject: Re: [PATCH V7 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT Date: Tue, 4 Apr 2017 09:13:28 -0400 Message-ID: <7c48ca8b-b834-3257-91dc-77e9d19def6c@codeaurora.org> References: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> Sender: linux-pci-owner@vger.kernel.org To: mayurkumar.patel@intel.com Cc: linux-pci@vger.kernel.org, timur@codeaurora.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-arm-msm@vger.kernel.org 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? 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Subject: Re: [PATCH V7 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT To: mayurkumar.patel@intel.com References: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> From: Sinan Kaya Message-ID: <7c48ca8b-b834-3257-91dc-77e9d19def6c@codeaurora.org> Date: Tue, 4 Apr 2017 09:13:28 -0400 MIME-Version: 1.0 In-Reply-To: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-pci@vger.kernel.org, timur@codeaurora.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: 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? 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: okaya@codeaurora.org (Sinan Kaya) Date: Tue, 4 Apr 2017 09:13:28 -0400 Subject: [PATCH V7 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT In-Reply-To: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> References: <1490880636-30542-1-git-send-email-okaya@codeaurora.org> Message-ID: <7c48ca8b-b834-3257-91dc-77e9d19def6c@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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? 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.