[PATCH v6] media: qcom: iris: move OPP handling to VPU power transaction

[PATCH v6] media: qcom: iris: move OPP handling to VPU power transaction

[Hungyu Lin]

The power-domain helpers currently update the OPP for core->dev while
enabling or disabling one functional power domain. This makes a
transaction-wide performance state depend on a low-level helper and can
leave the maximum OPP selected when a later power-domain resume fails.

Move the maximum OPP request to iris_vpu_power_on(), after the ICC vote
and before powering on the controller and hardware. Unwind the OPP vote
together with the ICC vote on failures.

Keep the power-domain helpers limited to runtime PM and use
pm_runtime_resume_and_get() so a failed resume does not leave the usage
counter incremented.

Fixes: bb8a95aa038e ("media: iris: implement power management")
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
Changes in v6:
- Move OPP handling from the power-domain helpers to
  iris_vpu_power_on().
- Request the maximum OPP before powering on the controller and
  hardware.
- Add OPP cleanup to the VPU power-on error path.
- Keep the power-domain helpers limited to runtime PM operations.
- Replace pm_runtime_get_sync() with pm_runtime_resume_and_get().
- Rename the patch to reflect the new approach.

Changes in v5:
- Add an error path to roll back the OPP vote when
  pm_runtime_resume_and_get() fails.

 .../media/platform/qcom/iris/iris_resources.c    | 16 +---------------
 .../media/platform/qcom/iris/iris_vpu_common.c   |  8 +++++++-
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
index df9b7badf0c6..5cccbf0636b0 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.c
+++ b/drivers/media/platform/qcom/iris/iris_resources.c
@@ -104,39 +104,25 @@ int iris_enable_power_domains(struct iris_core *core, enum platform_pm_domain_ty
 {
 	int pd_index = iris_get_pd_index_by_type(core, pd_type);
 	struct device *pd_dev;
-	int ret;
 
 	if (pd_index < 0)
 		return pd_index;
 
 	pd_dev = core->pmdomain_tbl->pd_devs[pd_index];
 
-	ret = iris_opp_set_rate(core->dev, ULONG_MAX);
-	if (ret)
-		return ret;
-
-	ret = pm_runtime_get_sync(pd_dev);
-	if (ret < 0)
-		return ret;
-
-	return ret;
+	return pm_runtime_resume_and_get(pd_dev);
 }
 
 int iris_disable_power_domains(struct iris_core *core, enum platform_pm_domain_type pd_type)
 {
 	int pd_index = iris_get_pd_index_by_type(core, pd_type);
 	struct device *pd_dev;
-	int ret;
 
 	if (pd_index < 0)
 		return pd_index;
 
 	pd_dev = core->pmdomain_tbl->pd_devs[pd_index];
 
-	ret = iris_opp_set_rate(core->dev, 0);
-	if (ret)
-		return ret;
-
 	pm_runtime_put_sync(pd_dev);
 
 	return 0;
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index ac9881e2cdc2..5209c32933db 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -472,10 +472,14 @@ int iris_vpu_power_on(struct iris_core *core)
 	if (ret)
 		goto err;
 
-	ret = core->iris_platform_data->vpu_ops->power_on_controller(core);
+	ret = iris_opp_set_rate(core->dev, ULONG_MAX);
 	if (ret)
 		goto err_unvote_icc;
 
+	ret = core->iris_platform_data->vpu_ops->power_on_controller(core);
+	if (ret)
+		goto err_opp;
+
 	ret = core->iris_platform_data->vpu_ops->power_on_hw(core);
 	if (ret)
 		goto err_power_off_ctrl;
@@ -495,6 +499,8 @@ int iris_vpu_power_on(struct iris_core *core)
 
 err_power_off_ctrl:
 	core->iris_platform_data->vpu_ops->power_off_controller(core);
+err_opp:
+	iris_opp_set_rate(core->dev, 0);
 err_unvote_icc:
 	iris_unset_icc_bw(core);
 err:
-- 
2.43.0

Re: [PATCH v6] media: qcom: iris: move OPP handling to VPU power transaction

[Konrad Dybcio]

On 7/29/26 3:10 AM, Hungyu Lin wrote:
> The power-domain helpers currently update the OPP for core->dev while
> enabling or disabling one functional power domain. This makes a
> transaction-wide performance state depend on a low-level helper and can
> leave the maximum OPP selected when a later power-domain resume fails.
> 
> Move the maximum OPP request to iris_vpu_power_on(), after the ICC vote
> and before powering on the controller and hardware. Unwind the OPP vote
> together with the ICC vote on failures.
> 
> Keep the power-domain helpers limited to runtime PM and use
> pm_runtime_resume_and_get() so a failed resume does not leave the usage
> counter incremented.
> 
> Fixes: bb8a95aa038e ("media: iris: implement power management")
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---

> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -472,10 +472,14 @@ int iris_vpu_power_on(struct iris_core *core)
>  	if (ret)
>  		goto err;
>  
> -	ret = core->iris_platform_data->vpu_ops->power_on_controller(core);
> +	ret = iris_opp_set_rate(core->dev, ULONG_MAX);
>  	if (ret)
>  		goto err_unvote_icc;
>  
> +	ret = core->iris_platform_data->vpu_ops->power_on_controller(core);
> +	if (ret)
> +		goto err_opp;
> +
>  	ret = core->iris_platform_data->vpu_ops->power_on_hw(core);
>  	if (ret)
>  		goto err_power_off_ctrl;
> @@ -495,6 +499,8 @@ int iris_vpu_power_on(struct iris_core *core)
>  
>  err_power_off_ctrl:
>  	core->iris_platform_data->vpu_ops->power_off_controller(core);
> +err_opp:
> +	iris_opp_set_rate(core->dev, 0);

iris_opp_set_rate(0) will almost never do what you want - it will set the
rate to something lower, but not disable the clocks or drop the power
vote. Dropping the vote will happen as the device (runtime) suspends.

This is an existing "issue" in the driver

Konrad