Messages in this thread | | | Date | Tue, 22 Aug 2023 09:57:41 +0530 | Subject | Re: [PATCH v3 3/3] PCI: qcom: Add OPP support for speed based performance state of RPMH | From | Krishna Chaitanya Chundru <> |
| |
On 8/22/2023 9:33 AM, Pavan Kondeti wrote: > On Mon, Aug 21, 2023 at 07:45:43PM +0530, Krishna chaitanya chundru wrote: >> Before link training vote for the maximum performance state of RPMH >> and once link is up, vote for the performance state based upon the link >> speed. >> >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 47 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 7a87a47..c57ca1a 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -22,6 +22,7 @@ >> #include <linux/of_device.h> >> #include <linux/of_gpio.h> >> #include <linux/pci.h> >> +#include <linux/pm_opp.h> >> #include <linux/pm_runtime.h> >> #include <linux/platform_device.h> >> #include <linux/phy/pcie.h> >> @@ -1357,6 +1358,32 @@ static int qcom_pcie_icc_init(struct qcom_pcie *pcie) >> return 0; >> } >> >> +static void qcom_pcie_opp_update(struct qcom_pcie *pcie) >> +{ >> + struct dw_pcie *pci = pcie->pci; >> + struct dev_pm_opp *opp; >> + u32 offset, status; >> + int speed, ret = 0; >> + >> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP); >> + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA); >> + >> + /* Only update constraints if link is up. */ >> + if (!(status & PCI_EXP_LNKSTA_DLLLA)) >> + return; >> + > What happens if link is not up during probe? We set max vote before > this, should not we bring it down in suspend and vote it back again in > resume?
ok, I will set to lower value in the suspend path if the link is not up. If the link is already up driver will not
do any modifications.
> >> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status); >> + >> + opp = dev_pm_opp_find_level_exact(pci->dev, speed); >> + if (!IS_ERR(opp)) { >> + ret = dev_pm_opp_set_opp(pci->dev, opp); >> + if (ret) >> + dev_err(pci->dev, "Failed to set opp: %d\n", ret); >> + dev_pm_opp_put(opp); >> + } > Since you added an error message, make it more useful by printing the > opp level also. dev_pm_opp_get_level(). Sure I will add this in next patch. > >> + >> +} >> + >> static void qcom_pcie_icc_update(struct qcom_pcie *pcie) >> { >> struct dw_pcie *pci = pcie->pci; >> @@ -1439,8 +1466,10 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie) >> static int qcom_pcie_probe(struct platform_device *pdev) >> { >> const struct qcom_pcie_cfg *pcie_cfg; >> + unsigned long max_freq = INT_MAX; >> struct device *dev = &pdev->dev; >> struct qcom_pcie *pcie; >> + struct dev_pm_opp *opp; >> struct dw_pcie_rp *pp; >> struct resource *res; >> struct dw_pcie *pci; >> @@ -1511,6 +1540,22 @@ static int qcom_pcie_probe(struct platform_device *pdev) >> if (ret) >> goto err_pm_runtime_put; >> >> + /* OPP table is optional */ >> + ret = devm_pm_opp_of_add_table(dev); >> + if (ret && ret != -ENODEV) { >> + dev_err(dev, "Invalid OPP table in Device tree\n"); >> + goto err_pm_runtime_put; >> + } >> + >> + /* vote for max level in the opp table */ >> + opp = dev_pm_opp_find_freq_floor(dev, &max_freq); >> + if (!IS_ERR(opp)) { >> + ret = dev_pm_opp_set_opp(dev, opp); >> + if (ret) >> + dev_err(pci->dev, "Failed to set opp: %d\n", ret); >> + dev_pm_opp_put(opp); >> + } >> + > This needs an update since you moved from frequency based voting to link > speed based voting.
dev_pm_opp_find_freq_floor will give us the max the opp level opp we don't have a similar API to get max opp-level
For that reason we are using this API.
- KC
> >> ret = pcie->cfg->ops->get_resources(pcie); >> if (ret) >> goto err_pm_runtime_put; >> @@ -1531,6 +1576,8 @@ static int qcom_pcie_probe(struct platform_device *pdev) >> >> qcom_pcie_icc_update(pcie); >> >> + qcom_pcie_opp_update(pcie); >> + >> if (pcie->mhi) >> qcom_pcie_init_debugfs(pcie); >> >> > Thanks, > Pavan
| |