Re: [PATCH v2 2/2] PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops

From: Konrad Dybcio
Date: Tue Oct 10 2023 - 12:34:10 EST




On 10/10/23 17:59, Manivannan Sadhasivam wrote:
ASPM is supported by Qcom host controllers/bridges on most of the recent
platforms and so the devices tested so far. But for enabling ASPM by
default (without Kconfig/cmdline/sysfs), BIOS has to enable ASPM on both
host bridge and downstream devices during boot. Unfortunately, none of the
BIOS available on Qcom platforms enables ASPM. Due to this, the platforms
making use of Qcom SoCs draw high power during runtime.

To fix this power issue, users/distros have to enable ASPM using configs
such as (Kconfig/cmdline/sysfs) or the BIOS has to start enabling ASPM.
The latter may happen in the future, but that won't address the issue on
current platforms. Also, asking users/distros to enable a feature to get
the power management right would provide an unpleasant out-of-the-box
experience.

So the apt solution is to enable ASPM in the controller driver itself. And
this is being accomplished by calling pci_enable_link_state() in the newly
introduced host_post_init() callback for all the devices connected to the
bus. This function enables all supported link low power states for both
host bridge and the downstream devices.

Due to limited testing, ASPM is only enabled for platforms making use of
ops_1_9_0 callbacks.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
---
[...]

+static int qcom_pcie_enable_aspm(struct pci_dev *pdev, void *userdata)
+{
+ /* Downstream devices need to be in D0 state before enabling PCI PM substates */
+ pci_set_power_state(pdev, PCI_D0);
+ pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
Do we not care about retval here?

+
+ return 0;
+}
+
+static void qcom_pcie_host_post_init_2_7_0(struct qcom_pcie *pcie)
post_init_enable_aspm?

Konrad