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@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
