[PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM

Konrad Dybcio posted 15 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM
Posted by Konrad Dybcio 2 years, 6 months ago
The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
ensure that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/gpucc-sm6115.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index ac048f7973d0..6fb84492d292 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -7,6 +7,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sm6115-gpucc.h>
@@ -442,10 +443,21 @@ MODULE_DEVICE_TABLE(of, gpu_cc_sm6115_match_table);
 static int gpu_cc_sm6115_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
+	int ret;
+
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		return ret;
+
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret)
+		return ret;
 
 	regmap = qcom_cc_map(pdev, &gpu_cc_sm6115_desc);
-	if (IS_ERR(regmap))
+	if (IS_ERR(regmap)) {
+		pm_runtime_put(&pdev->dev);
 		return PTR_ERR(regmap);
+	}
 
 	clk_alpha_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
 	clk_alpha_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);
@@ -465,7 +477,10 @@ static int gpu_cc_sm6115_probe(struct platform_device *pdev)
 	qcom_branch_set_clk_en(regmap, 0x1078);
 	qcom_branch_set_clk_en(regmap, 0x1060);
 
-	return qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
+	ret = qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
+	pm_runtime_put(&pdev->dev);
+
+	return ret;
 }
 
 static struct platform_driver gpu_cc_sm6115_driver = {

-- 
2.41.0
Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM
Posted by Johan Hovold 2 years, 6 months ago
On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
> ensure that it's enabled to prevent unwanted power collapse.

This bit is not correct, the power domain would not have been disabled
until you enable runtime PM as part of this very patch.

I noticed similar claims have incorrectly been made in the past, for
example, in the recent:

	2a541abd9837 clk: qcom: gcc-sc8280xp: Add runtime PM

and older:

	a91c483b42fa ("clk: qcom: videocc-sm8250: use runtime PM for the clock controller")

Johan
Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM
Posted by Konrad Dybcio 2 years, 6 months ago
On 18.07.2023 15:24, Johan Hovold wrote:
> On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
>> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
>> ensure that it's enabled to prevent unwanted power collapse.
> 
> This bit is not correct, the power domain would not have been disabled
> until you enable runtime PM as part of this very patch.
Right, this was a bit of a thought-jump. The part that ensures there's
any vote at all is actually the DT commit adding a reference to the
genpd.

Konrad
Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM
Posted by Konrad Dybcio 2 years, 6 months ago
On 18.07.2023 15:28, Konrad Dybcio wrote:
> On 18.07.2023 15:24, Johan Hovold wrote:
>> On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
>>> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
>>> ensure that it's enabled to prevent unwanted power collapse.
>>
>> This bit is not correct, the power domain would not have been disabled
>> until you enable runtime PM as part of this very patch.
> Right, this was a bit of a thought-jump. The part that ensures there's
> any vote at all is actually the DT commit adding a reference to the
> genpd.
Well I read it again and I think my original intention was "it = the power
domain" and not "it = runtime PM", it makes sense that way..

Anyway, I'll rephrase that to make it less ambiguous.

Konrad