Skip to content

[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?

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information