[PATCH v4 0/2] Use APIs in gdsc genpd to switch gdsc mode for venus v4 core

[PATCH v4 0/2] Use APIs in gdsc genpd to switch gdsc mode for venus v4 core

[Renjiang Han]

The Venus driver requires vcodec GDSC to be ON in SW mode for clock
operations and move it back to HW mode to gain power benefits. Earlier,
as there is no interface to switch the GDSC mode from GenPD framework,
the GDSC is moved to HW control mode as part of GDSC enable callback and
venus driver is writing to its POWER_CONTROL register to keep the GDSC ON
from SW whereever required. But the POWER_CONTROL register addresses are
not constant and can vary across the variants.

Also as per the HW recommendation, the GDSC mode switching needs to be
controlled from respective GDSC register and this is a uniform approach
across all the targets. Hence use dev_pm_genpd_set_hwmode() API which
controls GDSC mode switching using its respective GDSC register.

Make venus driver to use dev_pm_genpd_set_hwmode() to switch GDSC mode on
v4.
- 1. the venus driver adds compatibility with the new way to switch GDSC
mode.
- 2. the clock driver uses the HW_CTRL_TRIGGER flag, which means the venus
driver needs to use the dev_pm_genpd_set_hwmode() API to switch GDSC mode.

Validated this series on QCS615 and SC7180.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
Changes in v4:
- 1. Update the order of patches.
- 2. Update vcodec_control_v4 to try dev_pm_genpd_set_hwmode first.
- 3. Add hwmode_dev to indicate whether to use HW_CTRL_TRIGGER flag.
- 4. Update commit message and cover letter message.
- 5. Remove the patch that cleaned up dead code and will submit this patch
with next patch series.
- Link to v3: https://lore.kernel.org/r/20250115-switch_gdsc_mode-v3-0-9a24d2fd724c@quicinc.com

Changes in v3:
- 1. Update commit message.
- 2. Add a patch to clean up the dead code for the venus driver.
- 3. Remove vcodec_control_v4() function.
- 4. Directly call dev_pm_genpd_set_hwmode() without vcodec_control_v4().
- Link to v2: https://lore.kernel.org/r/20241223-switch_gdsc_mode-v2-0-eb5c96aee662@quicinc.com

Changes in v2:
- 1. Add the HW_CTRL_TRIGGER flag for the targets SM7150/SM8150 and SM8450
video GDSCs supporting movement between HW and SW mode of the GDSC.
(Suggested by Dmitry Baryshkov)
- 2. There is a dependency of the clock driver introducing the new flag
and the video driver adapting to this new API. Missing either the clock
and video driver could potentially break the video driver.
- Link to v1: https://lore.kernel.org/r/20241122-switch_gdsc_mode-v1-0-365f097ecbb0@quicinc.com

---
Renjiang Han (1):
      venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

Taniya Das (1):
      clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

 drivers/clk/qcom/videocc-sc7180.c              |  2 +-
 drivers/clk/qcom/videocc-sdm845.c              |  4 +--
 drivers/clk/qcom/videocc-sm7150.c              |  4 +--
 drivers/clk/qcom/videocc-sm8150.c              |  4 +--
 drivers/clk/qcom/videocc-sm8450.c              |  4 +--
 drivers/media/platform/qcom/venus/core.h       |  2 ++
 drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
 7 files changed, 32 insertions(+), 26 deletions(-)
---
base-commit: 63b3ff03d91ae8f875fe8747c781a521f78cde17
change-id: 20250115-switch_gdsc_mode-a9c14fad9a36

Best regards,
-- 
Renjiang Han <quic_renjiang@quicinc.com>

[PATCH v4 1/2] venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

[Renjiang Han]

There are two ways to switch GDSC mode. One is to write the POWER_CONTROL
register and the other is to use dev_pm_genpd_set_hwmode(). However, they
rely on different clock driver flags. dev_pm_genpd_set_hwmode() depends on
the HW_CTRL_TRIGGER flag and POWER_CONTROL register depends on the HW_CTRL
flag.

By default, the dev_pm_genpd_set_hwmode() is used to switch the GDSC mode.
If it fails and dev_pm_genpd_set_hwmode() returns -EOPNOTSUPP, it means
that the clock driver uses the HW_CTRL flag. At this time, the GDSC mode
is switched to write the POWER_CONTROL register.

Clock driver is using HW_CTRL_TRIGGER flag with V6. So hwmode_dev is
always true on using V6 platform. Conversely, if hwmode_dev is false, this
platform must be not using V6. Therefore, replace IS_V6 in poweroff_coreid
with hwmode_dev. Also, with HW_CTRL_TRIGGER flag, the vcodec gdsc gets
enabled in SW mode by default. Therefore, before disabling the GDSC, GDSC
should be switched to SW mode so that GDSC gets enabled in SW mode in the
next enable.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 drivers/media/platform/qcom/venus/core.h       |  2 ++
 drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 43532543292280be15adf688fc0c30f44e207c7f..0ccce89d3f54cf685ecce5b339a51e44f6ea3704 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -168,6 +168,7 @@ struct venus_format {
  * @root:	debugfs root directory
  * @venus_ver:	the venus firmware version
  * @dump_core:	a flag indicating that a core dump is required
+ * @hwmode_dev:	a flag indicating that HW_CTRL_TRIGGER is used in clock driver
  */
 struct venus_core {
 	void __iomem *base;
@@ -230,6 +231,7 @@ struct venus_core {
 		u32 rev;
 	} venus_ver;
 	unsigned long dump_core;
+	bool hwmode_dev;
 };
 
 struct vdec_controls {
diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..409aa9bd0b5d099c993eedb03177ec5ed918b4a0 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -412,9 +412,17 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
 	u32 val;
 	int ret;
 
-	if (IS_V6(core))
-		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
-	else if (coreid == VIDC_CORE_ID_1) {
+	ret = dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
+	if (ret == -EOPNOTSUPP) {
+		core->hwmode_dev = false;
+		goto legacy;
+	}
+
+	core->hwmode_dev = true;
+	return ret;
+
+legacy:
+	if (coreid == VIDC_CORE_ID_1) {
 		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
 		stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS;
 	} else {
@@ -450,7 +458,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
 
 		vcodec_clks_disable(core, core->vcodec0_clks);
 
-		if (!IS_V6(core)) {
+		if (!core->hwmode_dev) {
 			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
 			if (ret)
 				return ret;
@@ -468,7 +476,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
 
 		vcodec_clks_disable(core, core->vcodec1_clks);
 
-		if (!IS_V6(core)) {
+		if (!core->hwmode_dev) {
 			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
 			if (ret)
 				return ret;
@@ -491,11 +499,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
 		if (ret < 0)
 			return ret;
 
-		if (!IS_V6(core)) {
-			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
-			if (ret)
-				return ret;
-		}
+		ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
+		if (ret)
+			return ret;
 
 		ret = vcodec_clks_enable(core, core->vcodec0_clks);
 		if (ret)
@@ -511,11 +517,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
 		if (ret < 0)
 			return ret;
 
-		if (!IS_V6(core)) {
-			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
-			if (ret)
-				return ret;
-		}
+		ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
+		if (ret)
+			return ret;
 
 		ret = vcodec_clks_enable(core, core->vcodec1_clks);
 		if (ret)
@@ -811,7 +815,7 @@ static int vdec_power_v4(struct device *dev, int on)
 	else
 		vcodec_clks_disable(core, core->vcodec0_clks);
 
-	vcodec_control_v4(core, VIDC_CORE_ID_1, false);
+	ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
 
 	return ret;
 }
@@ -856,7 +860,7 @@ static int venc_power_v4(struct device *dev, int on)
 	else
 		vcodec_clks_disable(core, core->vcodec1_clks);
 
-	vcodec_control_v4(core, VIDC_CORE_ID_2, false);
+	ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
 
 	return ret;
 }

-- 
2.34.1

[PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Renjiang Han]

From: Taniya Das <quic_tdas@quicinc.com>

The video driver will be using the newly introduced
dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
control modes at runtime.
Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 drivers/clk/qcom/videocc-sc7180.c | 2 +-
 drivers/clk/qcom/videocc-sdm845.c | 4 ++--
 drivers/clk/qcom/videocc-sm7150.c | 4 ++--
 drivers/clk/qcom/videocc-sm8150.c | 4 ++--
 drivers/clk/qcom/videocc-sm8450.c | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/qcom/videocc-sc7180.c b/drivers/clk/qcom/videocc-sc7180.c
index d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134 100644
--- a/drivers/clk/qcom/videocc-sc7180.c
+++ b/drivers/clk/qcom/videocc-sc7180.c
@@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
 	.pd = {
 		.name = "vcodec0_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
diff --git a/drivers/clk/qcom/videocc-sdm845.c b/drivers/clk/qcom/videocc-sdm845.c
index f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5 100644
--- a/drivers/clk/qcom/videocc-sdm845.c
+++ b/drivers/clk/qcom/videocc-sdm845.c
@@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x890, 0x930 },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
@@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x8d0, 0x950 },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
diff --git a/drivers/clk/qcom/videocc-sm7150.c b/drivers/clk/qcom/videocc-sm7150.c
index 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780 100644
--- a/drivers/clk/qcom/videocc-sm7150.c
+++ b/drivers/clk/qcom/videocc-sm7150.c
@@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x890, 0x9ec },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
@@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
 	},
 	.cxcs = (unsigned int []){ 0x8d0, 0xa0c },
 	.cxc_count = 2,
-	.flags = HW_CTRL | POLL_CFG_GDSCR,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
 	.pd = {
 		.name = "vcodec0_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 
@@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
 	.pd = {
 		.name = "vcodec1_gdsc",
 	},
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER,
 	.pwrsts = PWRSTS_OFF_ON,
 };
 static struct clk_regmap *video_cc_sm8150_clocks[] = {
diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &video_cc_mvs0c_gdsc.pd,
-	.flags = RETAIN_FF_ENABLE | HW_CTRL,
+	.flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc video_cc_mvs1c_gdsc = {
@@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &video_cc_mvs1c_gdsc.pd,
-	.flags = RETAIN_FF_ENABLE | HW_CTRL,
+	.flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
 };
 
 static struct clk_regmap *video_cc_sm8450_clocks[] = {

-- 
2.34.1

Re: [PATCH v4 0/2] Use APIs in gdsc genpd to switch gdsc mode for venus v4 core

[Renjiang Han]

On 2/18/2025 6:33 PM, Renjiang Han wrote:
> The Venus driver requires vcodec GDSC to be ON in SW mode for clock
> operations and move it back to HW mode to gain power benefits. Earlier,
> as there is no interface to switch the GDSC mode from GenPD framework,
> the GDSC is moved to HW control mode as part of GDSC enable callback and
> venus driver is writing to its POWER_CONTROL register to keep the GDSC ON
> from SW whereever required. But the POWER_CONTROL register addresses are
> not constant and can vary across the variants.
>
> Also as per the HW recommendation, the GDSC mode switching needs to be
> controlled from respective GDSC register and this is a uniform approach
> across all the targets. Hence use dev_pm_genpd_set_hwmode() API which
> controls GDSC mode switching using its respective GDSC register.
>
> Make venus driver to use dev_pm_genpd_set_hwmode() to switch GDSC mode on
> v4.
> - 1. the venus driver adds compatibility with the new way to switch GDSC
> mode.
> - 2. the clock driver uses the HW_CTRL_TRIGGER flag, which means the venus
> driver needs to use the dev_pm_genpd_set_hwmode() API to switch GDSC mode.
>
> Validated this series on QCS615 and SC7180.
>
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
> Changes in v4:
> - 1. Update the order of patches.
> - 2. Update vcodec_control_v4 to try dev_pm_genpd_set_hwmode first.
> - 3. Add hwmode_dev to indicate whether to use HW_CTRL_TRIGGER flag.
> - 4. Update commit message and cover letter message.
> - 5. Remove the patch that cleaned up dead code and will submit this patch
> with next patch series.
> - Link to v3: https://lore.kernel.org/r/20250115-switch_gdsc_mode-v3-0-9a24d2fd724c@quicinc.com
>
> Changes in v3:
> - 1. Update commit message.
> - 2. Add a patch to clean up the dead code for the venus driver.
> - 3. Remove vcodec_control_v4() function.
> - 4. Directly call dev_pm_genpd_set_hwmode() without vcodec_control_v4().
> - Link to v2: https://lore.kernel.org/r/20241223-switch_gdsc_mode-v2-0-eb5c96aee662@quicinc.com
>
> Changes in v2:
> - 1. Add the HW_CTRL_TRIGGER flag for the targets SM7150/SM8150 and SM8450
> video GDSCs supporting movement between HW and SW mode of the GDSC.
> (Suggested by Dmitry Baryshkov)
> - 2. There is a dependency of the clock driver introducing the new flag
> and the video driver adapting to this new API. Missing either the clock
> and video driver could potentially break the video driver.
> - Link to v1: https://lore.kernel.org/r/20241122-switch_gdsc_mode-v1-0-365f097ecbb0@quicinc.com
>
> ---
> Renjiang Han (1):
>        venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4
>
> Taniya Das (1):
>        clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's
>
>   drivers/clk/qcom/videocc-sc7180.c              |  2 +-
>   drivers/clk/qcom/videocc-sdm845.c              |  4 +--
>   drivers/clk/qcom/videocc-sm7150.c              |  4 +--
>   drivers/clk/qcom/videocc-sm8150.c              |  4 +--
>   drivers/clk/qcom/videocc-sm8450.c              |  4 +--
>   drivers/media/platform/qcom/venus/core.h       |  2 ++
>   drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
>   7 files changed, 32 insertions(+), 26 deletions(-)
> ---
> base-commit: 63b3ff03d91ae8f875fe8747c781a521f78cde17
> change-id: 20250115-switch_gdsc_mode-a9c14fad9a36
Gentle ping on this patch series.
>
> Best regards,

-- 
Best Regards,
Renjiang

Re: [PATCH v4 1/2] venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

[Vikash Garodia]

On 2/18/2025 4:03 PM, Renjiang Han wrote:
> There are two ways to switch GDSC mode. One is to write the POWER_CONTROL
> register and the other is to use dev_pm_genpd_set_hwmode(). However, they
> rely on different clock driver flags. dev_pm_genpd_set_hwmode() depends on
> the HW_CTRL_TRIGGER flag and POWER_CONTROL register depends on the HW_CTRL
> flag.
> 
> By default, the dev_pm_genpd_set_hwmode() is used to switch the GDSC mode.
> If it fails and dev_pm_genpd_set_hwmode() returns -EOPNOTSUPP, it means
> that the clock driver uses the HW_CTRL flag. At this time, the GDSC mode
> is switched to write the POWER_CONTROL register.
> 
> Clock driver is using HW_CTRL_TRIGGER flag with V6. So hwmode_dev is
> always true on using V6 platform. Conversely, if hwmode_dev is false, this
> platform must be not using V6. Therefore, replace IS_V6 in poweroff_coreid
> with hwmode_dev. Also, with HW_CTRL_TRIGGER flag, the vcodec gdsc gets
> enabled in SW mode by default. Therefore, before disabling the GDSC, GDSC
> should be switched to SW mode so that GDSC gets enabled in SW mode in the
> next enable.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  drivers/media/platform/qcom/venus/core.h       |  2 ++
>  drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
>  2 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 43532543292280be15adf688fc0c30f44e207c7f..0ccce89d3f54cf685ecce5b339a51e44f6ea3704 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -168,6 +168,7 @@ struct venus_format {
>   * @root:	debugfs root directory
>   * @venus_ver:	the venus firmware version
>   * @dump_core:	a flag indicating that a core dump is required
> + * @hwmode_dev:	a flag indicating that HW_CTRL_TRIGGER is used in clock driver
>   */
>  struct venus_core {
>  	void __iomem *base;
> @@ -230,6 +231,7 @@ struct venus_core {
>  		u32 rev;
>  	} venus_ver;
>  	unsigned long dump_core;
> +	bool hwmode_dev;
>  };
>  
>  struct vdec_controls {
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..409aa9bd0b5d099c993eedb03177ec5ed918b4a0 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -412,9 +412,17 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
>  	u32 val;
>  	int ret;
>  
> -	if (IS_V6(core))
> -		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> -	else if (coreid == VIDC_CORE_ID_1) {
> +	ret = dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> +	if (ret == -EOPNOTSUPP) {
> +		core->hwmode_dev = false;
> +		goto legacy;
> +	}
> +
> +	core->hwmode_dev = true;
> +	return ret;
> +
> +legacy:
> +	if (coreid == VIDC_CORE_ID_1) {
>  		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
>  		stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS;
>  	} else {
> @@ -450,7 +458,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>  
>  		vcodec_clks_disable(core, core->vcodec0_clks);
>  
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>  			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>  			if (ret)
>  				return ret;
> @@ -468,7 +476,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>  
>  		vcodec_clks_disable(core, core->vcodec1_clks);
>  
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>  			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>  			if (ret)
>  				return ret;
> @@ -491,11 +499,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>  		if (ret < 0)
>  			return ret;
>  
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> +		if (ret)
> +			return ret;
>  
>  		ret = vcodec_clks_enable(core, core->vcodec0_clks);
>  		if (ret)
> @@ -511,11 +517,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>  		if (ret < 0)
>  			return ret;
>  
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> +		if (ret)
> +			return ret;
>  
>  		ret = vcodec_clks_enable(core, core->vcodec1_clks);
>  		if (ret)
> @@ -811,7 +815,7 @@ static int vdec_power_v4(struct device *dev, int on)
>  	else
>  		vcodec_clks_disable(core, core->vcodec0_clks);
>  
> -	vcodec_control_v4(core, VIDC_CORE_ID_1, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>  
>  	return ret;
>  }
> @@ -856,7 +860,7 @@ static int venc_power_v4(struct device *dev, int on)
>  	else
>  		vcodec_clks_disable(core, core->vcodec1_clks);
>  
> -	vcodec_control_v4(core, VIDC_CORE_ID_2, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>  
>  	return ret;
>  }

Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Re: [PATCH v4 1/2] venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

[Bryan O'Donoghue]

On 18/02/2025 10:33, Renjiang Han wrote:
> There are two ways to switch GDSC mode. One is to write the POWER_CONTROL
> register and the other is to use dev_pm_genpd_set_hwmode(). However, they
> rely on different clock driver flags. dev_pm_genpd_set_hwmode() depends on
> the HW_CTRL_TRIGGER flag and POWER_CONTROL register depends on the HW_CTRL
> flag.
> 
> By default, the dev_pm_genpd_set_hwmode() is used to switch the GDSC mode.
> If it fails and dev_pm_genpd_set_hwmode() returns -EOPNOTSUPP, it means
> that the clock driver uses the HW_CTRL flag. At this time, the GDSC mode
> is switched to write the POWER_CONTROL register.
> 
> Clock driver is using HW_CTRL_TRIGGER flag with V6. So hwmode_dev is
> always true on using V6 platform. Conversely, if hwmode_dev is false, this
> platform must be not using V6. Therefore, replace IS_V6 in poweroff_coreid
> with hwmode_dev. Also, with HW_CTRL_TRIGGER flag, the vcodec gdsc gets
> enabled in SW mode by default. Therefore, before disabling the GDSC, GDSC
> should be switched to SW mode so that GDSC gets enabled in SW mode in the
> next enable.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>   drivers/media/platform/qcom/venus/core.h       |  2 ++
>   drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
>   2 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 43532543292280be15adf688fc0c30f44e207c7f..0ccce89d3f54cf685ecce5b339a51e44f6ea3704 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -168,6 +168,7 @@ struct venus_format {
>    * @root:	debugfs root directory
>    * @venus_ver:	the venus firmware version
>    * @dump_core:	a flag indicating that a core dump is required
> + * @hwmode_dev:	a flag indicating that HW_CTRL_TRIGGER is used in clock driver
>    */
>   struct venus_core {
>   	void __iomem *base;
> @@ -230,6 +231,7 @@ struct venus_core {
>   		u32 rev;
>   	} venus_ver;
>   	unsigned long dump_core;
> +	bool hwmode_dev;
>   };
>   
>   struct vdec_controls {
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..409aa9bd0b5d099c993eedb03177ec5ed918b4a0 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -412,9 +412,17 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
>   	u32 val;
>   	int ret;
>   
> -	if (IS_V6(core))
> -		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> -	else if (coreid == VIDC_CORE_ID_1) {
> +	ret = dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> +	if (ret == -EOPNOTSUPP) {
> +		core->hwmode_dev = false;
> +		goto legacy;
> +	}
> +
> +	core->hwmode_dev = true;
> +	return ret;
> +
> +legacy:
> +	if (coreid == VIDC_CORE_ID_1) {
>   		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
>   		stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS;
>   	} else {
> @@ -450,7 +458,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>   
>   		vcodec_clks_disable(core, core->vcodec0_clks);
>   
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>   			if (ret)
>   				return ret;
> @@ -468,7 +476,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>   
>   		vcodec_clks_disable(core, core->vcodec1_clks);
>   
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>   			if (ret)
>   				return ret;
> @@ -491,11 +499,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>   		if (ret < 0)
>   			return ret;
>   
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> +		if (ret)
> +			return ret;
>   
>   		ret = vcodec_clks_enable(core, core->vcodec0_clks);
>   		if (ret)
> @@ -511,11 +517,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>   		if (ret < 0)
>   			return ret;
>   
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> +		if (ret)
> +			return ret;
>   
>   		ret = vcodec_clks_enable(core, core->vcodec1_clks);
>   		if (ret)
> @@ -811,7 +815,7 @@ static int vdec_power_v4(struct device *dev, int on)
>   	else
>   		vcodec_clks_disable(core, core->vcodec0_clks);
>   
> -	vcodec_control_v4(core, VIDC_CORE_ID_1, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>   
>   	return ret;
>   }
> @@ -856,7 +860,7 @@ static int venc_power_v4(struct device *dev, int on)
>   	else
>   		vcodec_clks_disable(core, core->vcodec1_clks);
>   
> -	vcodec_control_v4(core, VIDC_CORE_ID_2, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>   
>   	return ret;
>   }
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Bryan O'Donoghue]

On 18/02/2025 10:33, Renjiang Han wrote:
> From: Taniya Das <quic_tdas@quicinc.com>
> 
> The video driver will be using the newly introduced
> dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
> control modes at runtime.
> Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
> Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>   drivers/clk/qcom/videocc-sc7180.c | 2 +-
>   drivers/clk/qcom/videocc-sdm845.c | 4 ++--
>   drivers/clk/qcom/videocc-sm7150.c | 4 ++--
>   drivers/clk/qcom/videocc-sm8150.c | 4 ++--
>   drivers/clk/qcom/videocc-sm8450.c | 4 ++--
>   5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/qcom/videocc-sc7180.c b/drivers/clk/qcom/videocc-sc7180.c
> index d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134 100644
> --- a/drivers/clk/qcom/videocc-sc7180.c
> +++ b/drivers/clk/qcom/videocc-sc7180.c
> @@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
>   	.pd = {
>   		.name = "vcodec0_gdsc",
>   	},
> -	.flags = HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> diff --git a/drivers/clk/qcom/videocc-sdm845.c b/drivers/clk/qcom/videocc-sdm845.c
> index f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5 100644
> --- a/drivers/clk/qcom/videocc-sdm845.c
> +++ b/drivers/clk/qcom/videocc-sdm845.c
> @@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
>   	},
>   	.cxcs = (unsigned int []){ 0x890, 0x930 },
>   	.cxc_count = 2,
> -	.flags = HW_CTRL | POLL_CFG_GDSCR,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> @@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
>   	},
>   	.cxcs = (unsigned int []){ 0x8d0, 0x950 },
>   	.cxc_count = 2,
> -	.flags = HW_CTRL | POLL_CFG_GDSCR,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> diff --git a/drivers/clk/qcom/videocc-sm7150.c b/drivers/clk/qcom/videocc-sm7150.c
> index 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780 100644
> --- a/drivers/clk/qcom/videocc-sm7150.c
> +++ b/drivers/clk/qcom/videocc-sm7150.c
> @@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
>   	},
>   	.cxcs = (unsigned int []){ 0x890, 0x9ec },
>   	.cxc_count = 2,
> -	.flags = HW_CTRL | POLL_CFG_GDSCR,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> @@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
>   	},
>   	.cxcs = (unsigned int []){ 0x8d0, 0xa0c },
>   	.cxc_count = 2,
> -	.flags = HW_CTRL | POLL_CFG_GDSCR,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
> index daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b 100644
> --- a/drivers/clk/qcom/videocc-sm8150.c
> +++ b/drivers/clk/qcom/videocc-sm8150.c
> @@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
>   	.pd = {
>   		.name = "vcodec0_gdsc",
>   	},
> -	.flags = HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   
> @@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
>   	.pd = {
>   		.name = "vcodec1_gdsc",
>   	},
> -	.flags = HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER,
>   	.pwrsts = PWRSTS_OFF_ON,
>   };
>   static struct clk_regmap *video_cc_sm8150_clocks[] = {
> diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
> index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963 100644
> --- a/drivers/clk/qcom/videocc-sm8450.c
> +++ b/drivers/clk/qcom/videocc-sm8450.c
> @@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
>   	},
>   	.pwrsts = PWRSTS_OFF_ON,
>   	.parent = &video_cc_mvs0c_gdsc.pd,
> -	.flags = RETAIN_FF_ENABLE | HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>   };
>   
>   static struct gdsc video_cc_mvs1c_gdsc = {
> @@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
>   	},
>   	.pwrsts = PWRSTS_OFF_ON,
>   	.parent = &video_cc_mvs1c_gdsc.pd,
> -	.flags = RETAIN_FF_ENABLE | HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>   };
>   
>   static struct clk_regmap *video_cc_sm8450_clocks[] = {
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Renjiang Han]

On 3/19/2025 6:11 AM, Bryan O'Donoghue wrote:
> On 18/02/2025 10:33, Renjiang Han wrote:
>> From: Taniya Das <quic_tdas@quicinc.com>
>>
>> The video driver will be using the newly introduced
>> dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
>> control modes at runtime.
>> Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
>> Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
>>   drivers/clk/qcom/videocc-sc7180.c | 2 +-
>>   drivers/clk/qcom/videocc-sdm845.c | 4 ++--
>>   drivers/clk/qcom/videocc-sm7150.c | 4 ++--
>>   drivers/clk/qcom/videocc-sm8150.c | 4 ++--
>>   drivers/clk/qcom/videocc-sm8450.c | 4 ++--
>>   5 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/videocc-sc7180.c 
>> b/drivers/clk/qcom/videocc-sc7180.c
>> index 
>> d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134 
>> 100644
>> --- a/drivers/clk/qcom/videocc-sc7180.c
>> +++ b/drivers/clk/qcom/videocc-sc7180.c
>> @@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
>>       .pd = {
>>           .name = "vcodec0_gdsc",
>>       },
>> -    .flags = HW_CTRL,
>> +    .flags = HW_CTRL_TRIGGER,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   diff --git a/drivers/clk/qcom/videocc-sdm845.c 
>> b/drivers/clk/qcom/videocc-sdm845.c
>> index 
>> f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5 
>> 100644
>> --- a/drivers/clk/qcom/videocc-sdm845.c
>> +++ b/drivers/clk/qcom/videocc-sdm845.c
>> @@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
>>       },
>>       .cxcs = (unsigned int []){ 0x890, 0x930 },
>>       .cxc_count = 2,
>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   @@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
>>       },
>>       .cxcs = (unsigned int []){ 0x8d0, 0x950 },
>>       .cxc_count = 2,
>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   diff --git a/drivers/clk/qcom/videocc-sm7150.c 
>> b/drivers/clk/qcom/videocc-sm7150.c
>> index 
>> 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780 
>> 100644
>> --- a/drivers/clk/qcom/videocc-sm7150.c
>> +++ b/drivers/clk/qcom/videocc-sm7150.c
>> @@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
>>       },
>>       .cxcs = (unsigned int []){ 0x890, 0x9ec },
>>       .cxc_count = 2,
>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   @@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
>>       },
>>       .cxcs = (unsigned int []){ 0x8d0, 0xa0c },
>>       .cxc_count = 2,
>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   diff --git a/drivers/clk/qcom/videocc-sm8150.c 
>> b/drivers/clk/qcom/videocc-sm8150.c
>> index 
>> daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b 
>> 100644
>> --- a/drivers/clk/qcom/videocc-sm8150.c
>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>> @@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
>>       .pd = {
>>           .name = "vcodec0_gdsc",
>>       },
>> -    .flags = HW_CTRL,
>> +    .flags = HW_CTRL_TRIGGER,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   @@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
>>       .pd = {
>>           .name = "vcodec1_gdsc",
>>       },
>> -    .flags = HW_CTRL,
>> +    .flags = HW_CTRL_TRIGGER,
>>       .pwrsts = PWRSTS_OFF_ON,
>>   };
>>   static struct clk_regmap *video_cc_sm8150_clocks[] = {
>> diff --git a/drivers/clk/qcom/videocc-sm8450.c 
>> b/drivers/clk/qcom/videocc-sm8450.c
>> index 
>> f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963 
>> 100644
>> --- a/drivers/clk/qcom/videocc-sm8450.c
>> +++ b/drivers/clk/qcom/videocc-sm8450.c
>> @@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
>>       },
>>       .pwrsts = PWRSTS_OFF_ON,
>>       .parent = &video_cc_mvs0c_gdsc.pd,
>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>   };
>>     static struct gdsc video_cc_mvs1c_gdsc = {
>> @@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
>>       },
>>       .pwrsts = PWRSTS_OFF_ON,
>>       .parent = &video_cc_mvs1c_gdsc.pd,
>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>   };
>>     static struct clk_regmap *video_cc_sm8450_clocks[] = {
>>
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Hi @Bjorn

Could you help pick this into videocc?

-- 
Best Regards,
Renjiang

Re: [PATCH v4 1/2] venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

[Dmitry Baryshkov]

On Tue, Feb 18, 2025 at 04:03:20PM +0530, Renjiang Han wrote:
> There are two ways to switch GDSC mode. One is to write the POWER_CONTROL
> register and the other is to use dev_pm_genpd_set_hwmode(). However, they
> rely on different clock driver flags. dev_pm_genpd_set_hwmode() depends on
> the HW_CTRL_TRIGGER flag and POWER_CONTROL register depends on the HW_CTRL
> flag.
> 
> By default, the dev_pm_genpd_set_hwmode() is used to switch the GDSC mode.
> If it fails and dev_pm_genpd_set_hwmode() returns -EOPNOTSUPP, it means
> that the clock driver uses the HW_CTRL flag. At this time, the GDSC mode
> is switched to write the POWER_CONTROL register.
> 
> Clock driver is using HW_CTRL_TRIGGER flag with V6. So hwmode_dev is
> always true on using V6 platform. Conversely, if hwmode_dev is false, this
> platform must be not using V6. Therefore, replace IS_V6 in poweroff_coreid
> with hwmode_dev. Also, with HW_CTRL_TRIGGER flag, the vcodec gdsc gets
> enabled in SW mode by default. Therefore, before disabling the GDSC, GDSC
> should be switched to SW mode so that GDSC gets enabled in SW mode in the
> next enable.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  drivers/media/platform/qcom/venus/core.h       |  2 ++
>  drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
>  2 files changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 43532543292280be15adf688fc0c30f44e207c7f..0ccce89d3f54cf685ecce5b339a51e44f6ea3704 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -168,6 +168,7 @@ struct venus_format {
>   * @root:	debugfs root directory
>   * @venus_ver:	the venus firmware version
>   * @dump_core:	a flag indicating that a core dump is required
> + * @hwmode_dev:	a flag indicating that HW_CTRL_TRIGGER is used in clock driver
>   */
>  struct venus_core {
>  	void __iomem *base;
> @@ -230,6 +231,7 @@ struct venus_core {
>  		u32 rev;
>  	} venus_ver;
>  	unsigned long dump_core;
> +	bool hwmode_dev;
>  };
>  
>  struct vdec_controls {
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..409aa9bd0b5d099c993eedb03177ec5ed918b4a0 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -412,9 +412,17 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
>  	u32 val;
>  	int ret;
>  
> -	if (IS_V6(core))
> -		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> -	else if (coreid == VIDC_CORE_ID_1) {
> +	ret = dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> +	if (ret == -EOPNOTSUPP) {
> +		core->hwmode_dev = false;
> +		goto legacy;
> +	}
> +
> +	core->hwmode_dev = true;
> +	return ret;
> +
> +legacy:
> +	if (coreid == VIDC_CORE_ID_1) {
>  		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
>  		stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS;
>  	} else {
> @@ -450,7 +458,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>  
>  		vcodec_clks_disable(core, core->vcodec0_clks);
>  
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>  			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>  			if (ret)
>  				return ret;
> @@ -468,7 +476,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>  
>  		vcodec_clks_disable(core, core->vcodec1_clks);
>  
> -		if (!IS_V6(core)) {
> +		if (!core->hwmode_dev) {
>  			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>  			if (ret)
>  				return ret;
> @@ -491,11 +499,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>  		if (ret < 0)
>  			return ret;
>  
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
> +		if (ret)
> +			return ret;
>  
>  		ret = vcodec_clks_enable(core, core->vcodec0_clks);
>  		if (ret)
> @@ -511,11 +517,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>  		if (ret < 0)
>  			return ret;
>  
> -		if (!IS_V6(core)) {
> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> -			if (ret)
> -				return ret;
> -		}
> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
> +		if (ret)
> +			return ret;
>  
>  		ret = vcodec_clks_enable(core, core->vcodec1_clks);
>  		if (ret)
> @@ -811,7 +815,7 @@ static int vdec_power_v4(struct device *dev, int on)
>  	else
>  		vcodec_clks_disable(core, core->vcodec0_clks);
>  
> -	vcodec_control_v4(core, VIDC_CORE_ID_1, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);


return vcodec_control_v4(...);

>  
>  	return ret;
>  }
> @@ -856,7 +860,7 @@ static int venc_power_v4(struct device *dev, int on)
>  	else
>  		vcodec_clks_disable(core, core->vcodec1_clks);
>  
> -	vcodec_control_v4(core, VIDC_CORE_ID_2, false);
> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);

And here.

>  
>  	return ret;
>  }
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Dmitry Baryshkov]

On Mon, May 26, 2025 at 04:26:25PM +0800, Renjiang Han wrote:
> 
> On 3/19/2025 6:11 AM, Bryan O'Donoghue wrote:
> > On 18/02/2025 10:33, Renjiang Han wrote:
> > > From: Taniya Das <quic_tdas@quicinc.com>
> > > 
> > > The video driver will be using the newly introduced
> > > dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
> > > control modes at runtime.
> > > Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
> > > Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.
> > > 
> > > Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> > > ---
> > >   drivers/clk/qcom/videocc-sc7180.c | 2 +-
> > >   drivers/clk/qcom/videocc-sdm845.c | 4 ++--
> > >   drivers/clk/qcom/videocc-sm7150.c | 4 ++--
> > >   drivers/clk/qcom/videocc-sm8150.c | 4 ++--
> > >   drivers/clk/qcom/videocc-sm8450.c | 4 ++--
> > >   5 files changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/clk/qcom/videocc-sc7180.c
> > > b/drivers/clk/qcom/videocc-sc7180.c
> > > index d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134
> > > 100644
> > > --- a/drivers/clk/qcom/videocc-sc7180.c
> > > +++ b/drivers/clk/qcom/videocc-sc7180.c
> > > @@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
> > >       .pd = {
> > >           .name = "vcodec0_gdsc",
> > >       },
> > > -    .flags = HW_CTRL,
> > > +    .flags = HW_CTRL_TRIGGER,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   diff --git a/drivers/clk/qcom/videocc-sdm845.c
> > > b/drivers/clk/qcom/videocc-sdm845.c
> > > index f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5
> > > 100644
> > > --- a/drivers/clk/qcom/videocc-sdm845.c
> > > +++ b/drivers/clk/qcom/videocc-sdm845.c
> > > @@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
> > >       },
> > >       .cxcs = (unsigned int []){ 0x890, 0x930 },
> > >       .cxc_count = 2,
> > > -    .flags = HW_CTRL | POLL_CFG_GDSCR,
> > > +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   @@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
> > >       },
> > >       .cxcs = (unsigned int []){ 0x8d0, 0x950 },
> > >       .cxc_count = 2,
> > > -    .flags = HW_CTRL | POLL_CFG_GDSCR,
> > > +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   diff --git a/drivers/clk/qcom/videocc-sm7150.c
> > > b/drivers/clk/qcom/videocc-sm7150.c
> > > index 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780
> > > 100644
> > > --- a/drivers/clk/qcom/videocc-sm7150.c
> > > +++ b/drivers/clk/qcom/videocc-sm7150.c
> > > @@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
> > >       },
> > >       .cxcs = (unsigned int []){ 0x890, 0x9ec },
> > >       .cxc_count = 2,
> > > -    .flags = HW_CTRL | POLL_CFG_GDSCR,
> > > +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   @@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
> > >       },
> > >       .cxcs = (unsigned int []){ 0x8d0, 0xa0c },
> > >       .cxc_count = 2,
> > > -    .flags = HW_CTRL | POLL_CFG_GDSCR,
> > > +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   diff --git a/drivers/clk/qcom/videocc-sm8150.c
> > > b/drivers/clk/qcom/videocc-sm8150.c
> > > index daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b
> > > 100644
> > > --- a/drivers/clk/qcom/videocc-sm8150.c
> > > +++ b/drivers/clk/qcom/videocc-sm8150.c
> > > @@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
> > >       .pd = {
> > >           .name = "vcodec0_gdsc",
> > >       },
> > > -    .flags = HW_CTRL,
> > > +    .flags = HW_CTRL_TRIGGER,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   @@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
> > >       .pd = {
> > >           .name = "vcodec1_gdsc",
> > >       },
> > > -    .flags = HW_CTRL,
> > > +    .flags = HW_CTRL_TRIGGER,
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >   };
> > >   static struct clk_regmap *video_cc_sm8150_clocks[] = {
> > > diff --git a/drivers/clk/qcom/videocc-sm8450.c
> > > b/drivers/clk/qcom/videocc-sm8450.c
> > > index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963
> > > 100644
> > > --- a/drivers/clk/qcom/videocc-sm8450.c
> > > +++ b/drivers/clk/qcom/videocc-sm8450.c
> > > @@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
> > >       },
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >       .parent = &video_cc_mvs0c_gdsc.pd,
> > > -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
> > > +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
> > >   };
> > >     static struct gdsc video_cc_mvs1c_gdsc = {
> > > @@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
> > >       },
> > >       .pwrsts = PWRSTS_OFF_ON,
> > >       .parent = &video_cc_mvs1c_gdsc.pd,
> > > -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
> > > +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
> > >   };
> > >     static struct clk_regmap *video_cc_sm8450_clocks[] = {
> > > 
> > 
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 
> Hi @Bjorn
> 
> Could you help pick this into videocc?

This patch can not go if the venus patch hasn't been merged. Morover,
venus patch should directly preceed this one.

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Renjiang Han]

On 5/29/2025 3:30 AM, Dmitry Baryshkov wrote:
> On Mon, May 26, 2025 at 04:26:25PM +0800, Renjiang Han wrote:
>> On 3/19/2025 6:11 AM, Bryan O'Donoghue wrote:
>>> On 18/02/2025 10:33, Renjiang Han wrote:
>>>> From: Taniya Das <quic_tdas@quicinc.com>
>>>>
>>>> The video driver will be using the newly introduced
>>>> dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
>>>> control modes at runtime.
>>>> Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
>>>> Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.
>>>>
>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>> ---
>>>>    drivers/clk/qcom/videocc-sc7180.c | 2 +-
>>>>    drivers/clk/qcom/videocc-sdm845.c | 4 ++--
>>>>    drivers/clk/qcom/videocc-sm7150.c | 4 ++--
>>>>    drivers/clk/qcom/videocc-sm8150.c | 4 ++--
>>>>    drivers/clk/qcom/videocc-sm8450.c | 4 ++--
>>>>    5 files changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/videocc-sc7180.c
>>>> b/drivers/clk/qcom/videocc-sc7180.c
>>>> index d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134
>>>> 100644
>>>> --- a/drivers/clk/qcom/videocc-sc7180.c
>>>> +++ b/drivers/clk/qcom/videocc-sc7180.c
>>>> @@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>        .pd = {
>>>>            .name = "vcodec0_gdsc",
>>>>        },
>>>> -    .flags = HW_CTRL,
>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    diff --git a/drivers/clk/qcom/videocc-sdm845.c
>>>> b/drivers/clk/qcom/videocc-sdm845.c
>>>> index f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5
>>>> 100644
>>>> --- a/drivers/clk/qcom/videocc-sdm845.c
>>>> +++ b/drivers/clk/qcom/videocc-sdm845.c
>>>> @@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>        },
>>>>        .cxcs = (unsigned int []){ 0x890, 0x930 },
>>>>        .cxc_count = 2,
>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    @@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>        },
>>>>        .cxcs = (unsigned int []){ 0x8d0, 0x950 },
>>>>        .cxc_count = 2,
>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    diff --git a/drivers/clk/qcom/videocc-sm7150.c
>>>> b/drivers/clk/qcom/videocc-sm7150.c
>>>> index 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780
>>>> 100644
>>>> --- a/drivers/clk/qcom/videocc-sm7150.c
>>>> +++ b/drivers/clk/qcom/videocc-sm7150.c
>>>> @@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>        },
>>>>        .cxcs = (unsigned int []){ 0x890, 0x9ec },
>>>>        .cxc_count = 2,
>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    @@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>        },
>>>>        .cxcs = (unsigned int []){ 0x8d0, 0xa0c },
>>>>        .cxc_count = 2,
>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    diff --git a/drivers/clk/qcom/videocc-sm8150.c
>>>> b/drivers/clk/qcom/videocc-sm8150.c
>>>> index daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b
>>>> 100644
>>>> --- a/drivers/clk/qcom/videocc-sm8150.c
>>>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>>>> @@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>        .pd = {
>>>>            .name = "vcodec0_gdsc",
>>>>        },
>>>> -    .flags = HW_CTRL,
>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    @@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>        .pd = {
>>>>            .name = "vcodec1_gdsc",
>>>>        },
>>>> -    .flags = HW_CTRL,
>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>    };
>>>>    static struct clk_regmap *video_cc_sm8150_clocks[] = {
>>>> diff --git a/drivers/clk/qcom/videocc-sm8450.c
>>>> b/drivers/clk/qcom/videocc-sm8450.c
>>>> index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963
>>>> 100644
>>>> --- a/drivers/clk/qcom/videocc-sm8450.c
>>>> +++ b/drivers/clk/qcom/videocc-sm8450.c
>>>> @@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
>>>>        },
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>        .parent = &video_cc_mvs0c_gdsc.pd,
>>>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>>>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>>>    };
>>>>      static struct gdsc video_cc_mvs1c_gdsc = {
>>>> @@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
>>>>        },
>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>        .parent = &video_cc_mvs1c_gdsc.pd,
>>>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>>>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>>>    };
>>>>      static struct clk_regmap *video_cc_sm8450_clocks[] = {
>>>>
>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> Hi @Bjorn
>>
>> Could you help pick this into videocc?
> This patch can not go if the venus patch hasn't been merged. Morover,
> venus patch should directly preceed this one.
yes, venus patch has been merged into kernel.
>
-- 
Best Regards,
Renjiang

Re: [PATCH v4 1/2] venus: pm_helpers: add compatibility for dev_pm_genpd_set_hwmode on V4

[Renjiang Han]

On 5/29/2025 3:27 AM, Dmitry Baryshkov wrote:
> On Tue, Feb 18, 2025 at 04:03:20PM +0530, Renjiang Han wrote:
>> There are two ways to switch GDSC mode. One is to write the POWER_CONTROL
>> register and the other is to use dev_pm_genpd_set_hwmode(). However, they
>> rely on different clock driver flags. dev_pm_genpd_set_hwmode() depends on
>> the HW_CTRL_TRIGGER flag and POWER_CONTROL register depends on the HW_CTRL
>> flag.
>>
>> By default, the dev_pm_genpd_set_hwmode() is used to switch the GDSC mode.
>> If it fails and dev_pm_genpd_set_hwmode() returns -EOPNOTSUPP, it means
>> that the clock driver uses the HW_CTRL flag. At this time, the GDSC mode
>> is switched to write the POWER_CONTROL register.
>>
>> Clock driver is using HW_CTRL_TRIGGER flag with V6. So hwmode_dev is
>> always true on using V6 platform. Conversely, if hwmode_dev is false, this
>> platform must be not using V6. Therefore, replace IS_V6 in poweroff_coreid
>> with hwmode_dev. Also, with HW_CTRL_TRIGGER flag, the vcodec gdsc gets
>> enabled in SW mode by default. Therefore, before disabling the GDSC, GDSC
>> should be switched to SW mode so that GDSC gets enabled in SW mode in the
>> next enable.
>>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
>>   drivers/media/platform/qcom/venus/core.h       |  2 ++
>>   drivers/media/platform/qcom/venus/pm_helpers.c | 38 ++++++++++++++------------
>>   2 files changed, 23 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>> index 43532543292280be15adf688fc0c30f44e207c7f..0ccce89d3f54cf685ecce5b339a51e44f6ea3704 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -168,6 +168,7 @@ struct venus_format {
>>    * @root:	debugfs root directory
>>    * @venus_ver:	the venus firmware version
>>    * @dump_core:	a flag indicating that a core dump is required
>> + * @hwmode_dev:	a flag indicating that HW_CTRL_TRIGGER is used in clock driver
>>    */
>>   struct venus_core {
>>   	void __iomem *base;
>> @@ -230,6 +231,7 @@ struct venus_core {
>>   		u32 rev;
>>   	} venus_ver;
>>   	unsigned long dump_core;
>> +	bool hwmode_dev;
>>   };
>>   
>>   struct vdec_controls {
>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>> index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..409aa9bd0b5d099c993eedb03177ec5ed918b4a0 100644
>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>> @@ -412,9 +412,17 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
>>   	u32 val;
>>   	int ret;
>>   
>> -	if (IS_V6(core))
>> -		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
>> -	else if (coreid == VIDC_CORE_ID_1) {
>> +	ret = dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
>> +	if (ret == -EOPNOTSUPP) {
>> +		core->hwmode_dev = false;
>> +		goto legacy;
>> +	}
>> +
>> +	core->hwmode_dev = true;
>> +	return ret;
>> +
>> +legacy:
>> +	if (coreid == VIDC_CORE_ID_1) {
>>   		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
>>   		stat = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_STATUS;
>>   	} else {
>> @@ -450,7 +458,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>>   
>>   		vcodec_clks_disable(core, core->vcodec0_clks);
>>   
>> -		if (!IS_V6(core)) {
>> +		if (!core->hwmode_dev) {
>>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>>   			if (ret)
>>   				return ret;
>> @@ -468,7 +476,7 @@ static int poweroff_coreid(struct venus_core *core, unsigned int coreid_mask)
>>   
>>   		vcodec_clks_disable(core, core->vcodec1_clks);
>>   
>> -		if (!IS_V6(core)) {
>> +		if (!core->hwmode_dev) {
>>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>>   			if (ret)
>>   				return ret;
>> @@ -491,11 +499,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>>   		if (ret < 0)
>>   			return ret;
>>   
>> -		if (!IS_V6(core)) {
>> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
>> -			if (ret)
>> -				return ret;
>> -		}
>> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_1, true);
>> +		if (ret)
>> +			return ret;
>>   
>>   		ret = vcodec_clks_enable(core, core->vcodec0_clks);
>>   		if (ret)
>> @@ -511,11 +517,9 @@ static int poweron_coreid(struct venus_core *core, unsigned int coreid_mask)
>>   		if (ret < 0)
>>   			return ret;
>>   
>> -		if (!IS_V6(core)) {
>> -			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
>> -			if (ret)
>> -				return ret;
>> -		}
>> +		ret = vcodec_control_v4(core, VIDC_CORE_ID_2, true);
>> +		if (ret)
>> +			return ret;
>>   
>>   		ret = vcodec_clks_enable(core, core->vcodec1_clks);
>>   		if (ret)
>> @@ -811,7 +815,7 @@ static int vdec_power_v4(struct device *dev, int on)
>>   	else
>>   		vcodec_clks_disable(core, core->vcodec0_clks);
>>   
>> -	vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>
> return vcodec_control_v4(...);
OK, thanks for your comments. This patch has already been merged. I have
another patch that cleans up code and removes dead code, but I haven’t
submitted it yet. It depends on the videocc flag, so I plan to wait until
the videocc patch is picked before submitting it.
>
>>   
>>   	return ret;
>>   }
>> @@ -856,7 +860,7 @@ static int venc_power_v4(struct device *dev, int on)
>>   	else
>>   		vcodec_clks_disable(core, core->vcodec1_clks);
>>   
>> -	vcodec_control_v4(core, VIDC_CORE_ID_2, false);
>> +	ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
> And here.
>
>>   
>>   	return ret;
>>   }
>>
>> -- 
>> 2.34.1
>>
-- 
Best Regards,
Renjiang

Re: [PATCH v4 2/2] clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's

[Vikash Garodia]

On 5/29/2025 7:23 AM, Renjiang Han wrote:
> 
> On 5/29/2025 3:30 AM, Dmitry Baryshkov wrote:
>> On Mon, May 26, 2025 at 04:26:25PM +0800, Renjiang Han wrote:
>>> On 3/19/2025 6:11 AM, Bryan O'Donoghue wrote:
>>>> On 18/02/2025 10:33, Renjiang Han wrote:
>>>>> From: Taniya Das <quic_tdas@quicinc.com>
>>>>>
>>>>> The video driver will be using the newly introduced
>>>>> dev_pm_genpd_set_hwmode() API to switch the video GDSC to HW and SW
>>>>> control modes at runtime.
>>>>> Hence use HW_CTRL_TRIGGER flag instead of HW_CTRL for video GDSC's for
>>>>> Qualcomm SoC SC7180, SDM845, SM7150, SM8150 and SM8450.
>>>>>
>>>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>>> ---
>>>>>    drivers/clk/qcom/videocc-sc7180.c | 2 +-
>>>>>    drivers/clk/qcom/videocc-sdm845.c | 4 ++--
>>>>>    drivers/clk/qcom/videocc-sm7150.c | 4 ++--
>>>>>    drivers/clk/qcom/videocc-sm8150.c | 4 ++--
>>>>>    drivers/clk/qcom/videocc-sm8450.c | 4 ++--
>>>>>    5 files changed, 9 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/qcom/videocc-sc7180.c
>>>>> b/drivers/clk/qcom/videocc-sc7180.c
>>>>> index
>>>>> d7f84548039699ce6fdd7c0f6675c168d5eaf4c1..dd2441d6aa83bd7cff17deeb42f5d011c1e9b134
>>>>> 100644
>>>>> --- a/drivers/clk/qcom/videocc-sc7180.c
>>>>> +++ b/drivers/clk/qcom/videocc-sc7180.c
>>>>> @@ -166,7 +166,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>>        .pd = {
>>>>>            .name = "vcodec0_gdsc",
>>>>>        },
>>>>> -    .flags = HW_CTRL,
>>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    diff --git a/drivers/clk/qcom/videocc-sdm845.c
>>>>> b/drivers/clk/qcom/videocc-sdm845.c
>>>>> index
>>>>> f77a0777947773dc8902c92098acff71b9b8f10f..6dedc80a8b3e18eca82c08a5bcd7e1fdc374d4b5
>>>>> 100644
>>>>> --- a/drivers/clk/qcom/videocc-sdm845.c
>>>>> +++ b/drivers/clk/qcom/videocc-sdm845.c
>>>>> @@ -260,7 +260,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>>        },
>>>>>        .cxcs = (unsigned int []){ 0x890, 0x930 },
>>>>>        .cxc_count = 2,
>>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    @@ -271,7 +271,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>>        },
>>>>>        .cxcs = (unsigned int []){ 0x8d0, 0x950 },
>>>>>        .cxc_count = 2,
>>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    diff --git a/drivers/clk/qcom/videocc-sm7150.c
>>>>> b/drivers/clk/qcom/videocc-sm7150.c
>>>>> index
>>>>> 14ef7f5617537363673662adc3910ddba8ea6a4f..b6912560ef9b7a84e7fd1d9924f5aac6967da780
>>>>> 100644
>>>>> --- a/drivers/clk/qcom/videocc-sm7150.c
>>>>> +++ b/drivers/clk/qcom/videocc-sm7150.c
>>>>> @@ -271,7 +271,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>>        },
>>>>>        .cxcs = (unsigned int []){ 0x890, 0x9ec },
>>>>>        .cxc_count = 2,
>>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    @@ -282,7 +282,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>>        },
>>>>>        .cxcs = (unsigned int []){ 0x8d0, 0xa0c },
>>>>>        .cxc_count = 2,
>>>>> -    .flags = HW_CTRL | POLL_CFG_GDSCR,
>>>>> +    .flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    diff --git a/drivers/clk/qcom/videocc-sm8150.c
>>>>> b/drivers/clk/qcom/videocc-sm8150.c
>>>>> index
>>>>> daab3237eec19b727d34512d3a2ba1d7bd2743d6..3024f6fc89c8b374f2ef13debc283998cb136f6b
>>>>> 100644
>>>>> --- a/drivers/clk/qcom/videocc-sm8150.c
>>>>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>>>>> @@ -179,7 +179,7 @@ static struct gdsc vcodec0_gdsc = {
>>>>>        .pd = {
>>>>>            .name = "vcodec0_gdsc",
>>>>>        },
>>>>> -    .flags = HW_CTRL,
>>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    @@ -188,7 +188,7 @@ static struct gdsc vcodec1_gdsc = {
>>>>>        .pd = {
>>>>>            .name = "vcodec1_gdsc",
>>>>>        },
>>>>> -    .flags = HW_CTRL,
>>>>> +    .flags = HW_CTRL_TRIGGER,
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>    };
>>>>>    static struct clk_regmap *video_cc_sm8150_clocks[] = {
>>>>> diff --git a/drivers/clk/qcom/videocc-sm8450.c
>>>>> b/drivers/clk/qcom/videocc-sm8450.c
>>>>> index
>>>>> f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..4cefcbbc020f201f19c75c20229415e0bdea2963
>>>>> 100644
>>>>> --- a/drivers/clk/qcom/videocc-sm8450.c
>>>>> +++ b/drivers/clk/qcom/videocc-sm8450.c
>>>>> @@ -347,7 +347,7 @@ static struct gdsc video_cc_mvs0_gdsc = {
>>>>>        },
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>        .parent = &video_cc_mvs0c_gdsc.pd,
>>>>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>>>>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>>>>    };
>>>>>      static struct gdsc video_cc_mvs1c_gdsc = {
>>>>> @@ -372,7 +372,7 @@ static struct gdsc video_cc_mvs1_gdsc = {
>>>>>        },
>>>>>        .pwrsts = PWRSTS_OFF_ON,
>>>>>        .parent = &video_cc_mvs1c_gdsc.pd,
>>>>> -    .flags = RETAIN_FF_ENABLE | HW_CTRL,
>>>>> +    .flags = HW_CTRL_TRIGGER | RETAIN_FF_ENABLE,
>>>>>    };
>>>>>      static struct clk_regmap *video_cc_sm8450_clocks[] = {
>>>>>
>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> Hi @Bjorn
>>>
>>> Could you help pick this into videocc?
>> This patch can not go if the venus patch hasn't been merged. Morover,
>> venus patch should directly preceed this one.
> yes, venus patch has been merged into kernel.
Better to drop the video driver patch which is merged and put the new version.
This would avoid such confusion.

Also add my RB tag
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Regards,
Vikash