[powersave] (e)DP PHY missing runtime PM
According to /sys/kernel/debug/pm_genpd/pm_genpd_summary the displayport-controllers were holding up the MMCX rail from suspending.
I quickly added some basic runtime_pm calls, only thinking of not holding up the rest of the system:
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index ca9bb9d70e..a93eec5b9c 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -16,6 +16,7 @@
#include <linux/phy/phy.h>
#include <linux/phy/phy-dp.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/slab.h>
@@ -748,9 +749,13 @@
u32 val;
u8 cfg1;
+ ret = pm_runtime_resume_and_get(edp->dev);
+ if (ret < 0)
+ return ret;
+
ret = edp->cfg->ver_ops->com_power_on(edp);
if (ret)
- return ret;
+ goto rpm_put;
if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
ldo_config = 0x1;
@@ -763,12 +768,12 @@
if (edp->dp_opts.ssc) {
ret = qcom_edp_configure_ssc(edp);
if (ret)
- return ret;
+ goto rpm_put;
}
ret = qcom_edp_configure_pll(edp);
if (ret)
- return ret;
+ goto rpm_put;
/* TX Lane configuration */
writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
@@ -790,7 +795,7 @@
ret = qcom_edp_set_vco_div(edp, &pixel_freq);
if (ret)
- return ret;
+ goto rpm_put;
writel(0x01, edp->edp + DP_PHY_CFG);
writel(0x05, edp->edp + DP_PHY_CFG);
@@ -799,7 +804,7 @@
ret = edp->cfg->ver_ops->com_resetsm_cntrl(edp);
if (ret)
- return ret;
+ goto rpm_put;
writel(0x19, edp->edp + DP_PHY_CFG);
writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
@@ -854,12 +859,14 @@
ret = readl_poll_timeout(edp->edp + DP_PHY_STATUS,
val, val & BIT(1), 500, 10000);
if (ret)
- return ret;
+ goto rpm_put;
clk_set_rate(edp->dp_link_hw.clk, edp->dp_opts.link_rate * 100000);
clk_set_rate(edp->dp_pixel_hw.clk, pixel_freq);
- return 0;
+rpm_put:
+ pm_runtime_put(edp->dev);
+ return ret;
}
static int qcom_edp_phy_power_off(struct phy *phy)
@@ -868,6 +875,7 @@
writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
+ pm_runtime_put(edp->dev);
return 0;
}
@@ -1127,11 +1135,19 @@
}
phy_set_drvdata(edp->phy, edp);
+ platform_set_drvdata(pdev, edp);
+ pm_runtime_enable(dev);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
}
+static void qcom_edp_phy_remove(struct platform_device *pdev) {
+ struct qcom_edp *edp = platform_get_drvdata(pdev);
+
+ pm_runtime_disable(edp->dev);
+}
+
static const struct of_device_id qcom_edp_phy_match_table[] = {
{ .compatible = "qcom,sa8775p-edp-phy", .data = &sa8775p_dp_phy_cfg, },
{ .compatible = "qcom,sc7280-edp-phy", .data = &sc7280_dp_phy_cfg, },
@@ -1145,6 +1161,7 @@
static struct platform_driver qcom_edp_phy_driver = {
.probe = qcom_edp_phy_probe,
+ .remove = qcom_edp_phy_remove,
.driver = {
.name = "qcom-edp-phy",
.of_match_table = qcom_edp_phy_match_table,
Just now I found that @abelvesa posted a patch a while ago that received a lot of feedback, but it seems like a v2 wasn't posted. In that patch, SET_RUNTIME_PM_OPS is used. Is that necessary? Does power_on/off happen around each frame transmission, or only broadly when the entire display is on/off?