linux-kernel - Re: [PATCH v2 2/2] venus: pm_helpers: Use dev_pm_genpd_set

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4d3a2d5a-3791-4bc8-a43d-6087e8c35619@quicinc.com>
Date: Thu, 2 Jan 2025 11:47:22 +0800
From: Renjiang Han <quic_renjiang@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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 v2 2/2] venus: pm_helpers: Use dev_pm_genpd_set_hwmode to
 switch GDSC mode on V4


On 12/23/2024 7:41 PM, Dmitry Baryshkov wrote:
>> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
>> index 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..a2062b366d4aedba3eb5e4be456a005847eaec0b 100644
>> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
>> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
>> @@ -412,7 +412,7 @@ static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool enable)
>>   	u32 val;
>>   	int ret;
>>   
>> -	if (IS_V6(core))
>> +	if (IS_V6(core) || IS_V4(core))
>>   		return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], !enable);
> It is being called only for v4 and v6 targets. Drop the rest of the
> function and inline the result. I'd suggest keeping it as two patches
> though: this one which adds IS_V4() all over the place and the next one
> which performs cleanup of the dead code.
  Thanks for your comment. poweron_coreid(), poweroff_coreid() and
  vcodec_control_v4() are called only for v4 and v6. I will clean up
  the dead code with another patch.
>
>>   	else if (coreid == VIDC_CORE_ID_1) {
>>   		ctrl = core->wrapper_base + WRAPPER_VCODEC0_MMCC_POWER_CONTROL;
>> @@ -450,7 +450,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 (!IS_V6(core) && !IS_V4(core)) {
>>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_1, false);
>>   			if (ret)
>>   				return ret;
>> @@ -468,7 +468,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 (!IS_V6(core) && !IS_V4(core)) {
>>   			ret = vcodec_control_v4(core, VIDC_CORE_ID_2, false);
> The poweron_coreid() and poweroff_coreid() functions are called only for
> v4 and v6. The v6 case was masked out earlier. Now you've removed the v4
> case too. Can we drop such vcodec_control_v4() calls completely?

  I cleaned up the code, and finally vcodec_control_v4() looks like this.

static int vcodec_control_v4(struct venus_core *core, u32 coreid, bool 
enable)
{
     return dev_pm_genpd_set_hwmode(core->pmdomains->pd_devs[coreid], 
!enable);

}

  Functionally, we can drop vcodec_control_v4(), but architecturally, I
  don’t recommend removing this function, because I think it’s easier to
  read the code with this function.

-- 
Best Regards,
Renjiang