Re: [PATCH v1] PCI: qcom: Add sysfs entry to change link speed dynamically

From: Krishna Chaitanya Chundru
Date: Thu Aug 17 2023 - 22:27:26 EST


On 8/17/2023 10:50 PM, Bjorn Helgaas wrote:
On Thu, Aug 17, 2023 at 08:04:43AM +0530, Krishna chaitanya chundru wrote:
PCIe can operate on lower GEN speed if client decided based upon
the bandwidth & latency requirements. To support dynamic GEN speed
switch adding this sysfs support.
Who does "client" refer to? I assume it's the system administrator,
but of course the endpoint is involved in the hardware speed
negotiation, so one could think of the endpoint as a "client" with its
own speed capabilities and requirements.

The client here refers to either system administrator or endpoint both can decide to go

to lower gen speed for reducing power consumption.

To change the GEN speed the link should be in L0, so first disable
L0s & L1.

L0s needs to be disabled at both RC & EP because L0s entry is
independent. For enabling L0s both ends of the link needs to support
it, so first check if L0s is supported on both ends and then enable
L0s.
Is there a place to document this sysfs knob? Why should it be
qcom-specific? This sounds like generic PCIe functionality.

The ASPM stuff looks like it should be done by aspm.c, not done behind
its back.

Everything here looks generic (not qcom-specific) except the
qcom_pcie_icc_update() and qcom_pcie_opp_update(). Maybe we need some
core infrastructure around this.

I will try to move the logic to the pci generic and add infrastructure to call controller specific calls.

- KC


This patch is dependent on "PCI: qcom: Add support for OPP"
https://lore.kernel.org/linux-arm-msm/1692192264-18515-1-git-send-email-quic_krichai@xxxxxxxxxxx/T/#t

Signed-off-by: Krishna chaitanya chundru <quic_krichai@xxxxxxxxxxx>
---
drivers/pci/controller/dwc/pcie-qcom.c | 141 +++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 831d158..ad67d17 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -241,10 +241,150 @@ struct qcom_pcie {
const struct qcom_pcie_cfg *cfg;
struct dentry *debugfs;
bool suspended;
+ bool l0s_supported;
};
#define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
+static void qcom_pcie_icc_update(struct qcom_pcie *pcie);
+static void qcom_pcie_opp_update(struct qcom_pcie *pcie);
+
+static int qcom_pcie_disable_l0s(struct pci_dev *pdev, void *userdata)
+{
+ int lnkctl;
+
+ pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCTL, &lnkctl);
+ lnkctl &= ~(PCI_EXP_LNKCTL_ASPM_L0S);
+ pci_write_config_word(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCTL, lnkctl);
+
+ return 0;
+}
+
+static int qcom_pcie_check_l0s_support(struct pci_dev *pdev, void *userdata)
+{
+ struct pci_dev *parent = pdev->bus->self;
+ struct qcom_pcie *pcie = userdata;
+ struct dw_pcie *pci = pcie->pci;
+ int lnkcap;
+
+ /* check parent supports L0s */
+ if (parent) {
+ dev_err(pci->dev, "parent\n");
+ pci_read_config_dword(parent, pci_pcie_cap(parent) + PCI_EXP_LNKCAP,
+ &lnkcap);
+ if (!(lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)) {
+ dev_info(pci->dev, "Parent does not support L0s\n");
+ pcie->l0s_supported = false;
+ return 0;
+ }
+ }
+
+ pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCAP,
+ &lnkcap);
+ dev_err(pci->dev, "child %x\n", lnkcap);
+ if (!(lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)) {
+ dev_info(pci->dev, "Device does not support L0s\n");
+ pcie->l0s_supported = false;
+ return 0;
+ }
+
+ return 0;
+}
+
+static int qcom_pcie_enable_l0s(struct pci_dev *pdev, void *userdata)
+{
+ int lnkctl;
+
+ pci_read_config_dword(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCTL, &lnkctl);
+ lnkctl |= (PCI_EXP_LNKCTL_ASPM_L0S);
+ pci_write_config_word(pdev, pci_pcie_cap(pdev) + PCI_EXP_LNKCTL, lnkctl);
+
+ return 0;
+}
+
+static ssize_t qcom_pcie_speed_change_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ unsigned int current_speed, target_speed, max_speed;
+ struct qcom_pcie *pcie = dev_get_drvdata(dev);
+ struct pci_bus *child, *root_bus = NULL;
+ struct dw_pcie_rp *pp = &pcie->pci->pp;
+ struct dw_pcie *pci = pcie->pci;
+ struct pci_dev *pdev;
+ u16 offset;
+ u32 val;
+ int ret;
+
+ list_for_each_entry(child, &pp->bridge->bus->children, node) {
+ if (child->parent == pp->bridge->bus) {
+ root_bus = child;
+ break;
+ }
+ }
+
+ pdev = root_bus->self;
+
+ offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+
+ val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP);
+ max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
+
+ val = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+ current_speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val);
+
+ ret = kstrtouint(buf, 10, &target_speed);
+ if (ret)
+ return ret;
+
+ if (target_speed > max_speed)
+ return -EINVAL;
+
+ if (current_speed == target_speed)
+ return count;
+
+ pci_walk_bus(pp->bridge->bus, qcom_pcie_disable_l0s, pcie);
+
+ /* Disable L1 */
+ val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL);
+ val &= ~(PCI_EXP_LNKCTL_ASPM_L1);
+ dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL, val);
+
+ /* Set target GEN speed */
+ val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
+ val &= ~PCI_EXP_LNKCTL2_TLS;
+ dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, val | target_speed);
+
+ ret = pcie_retrain_link(pdev, true);
+ if (ret)
+ dev_err(dev, "Link retrain failed %d\n", ret);
+
+ /* Enable L1 */
+ val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL);
+ val |= (PCI_EXP_LNKCTL_ASPM_L1);
+ dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL, val);
+
+ pcie->l0s_supported = true;
+ pci_walk_bus(pp->bridge->bus, qcom_pcie_check_l0s_support, pcie);
+
+ if (pcie->l0s_supported)
+ pci_walk_bus(pp->bridge->bus, qcom_pcie_enable_l0s, pcie);
+
+ qcom_pcie_icc_update(pcie);
+
+ qcom_pcie_opp_update(pcie);
+
+ return count;
+}
+static DEVICE_ATTR_WO(qcom_pcie_speed_change);
+
+static struct attribute *qcom_pcie_attrs[] = {
+ &dev_attr_qcom_pcie_speed_change.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(qcom_pcie);
+
static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
{
gpiod_set_value_cansleep(pcie->reset, 1);
@@ -1716,6 +1856,7 @@ static struct platform_driver qcom_pcie_driver = {
.of_match_table = qcom_pcie_match,
.pm = &qcom_pcie_pm_ops,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .dev_groups = qcom_pcie_groups,
},
};
builtin_platform_driver(qcom_pcie_driver);
--
2.7.4