| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ee143088-89fb-4d4e-bc47-816e90dc7fa7@quicinc.com>
Date: Thu, 29 May 2025 10:03:53 +0800
From: Renjiang Han <quic_renjiang@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: Bjorn Andersson <andersson@...nel.org>,
Michael Turquette
<mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Stanimir Varbanov
<stanimir.k.varbanov@...il.com>,
Vikash Garodia <quic_vgarodia@...cinc.com>,
Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Mauro Carvalho Chehab
<mchehab@...nel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] venus: pm_helpers: add compatibility for
dev_pm_genpd_set_hwmode on V4
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@...cinc.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
Powered by blists - more mailing lists