Received-SPF: pass (google.com: domain of linux-kernel+bounces-101088-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <8bfcacb1-b97a-8c54-7e41-f66df8ac32fd@huawei.com>
Date: Wed, 13 Mar 2024 11:57:43 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.0
From: Yipeng Zou <zouyipeng@huawei.com>
Subject: Re: [PATCH V3] cpufreq: Fix per-policy boost behavior on SoCs using
 cpufreq_boost_set_sw
To: Sibi Sankar <quic_sibis@quicinc.com>, <dietmar.eggemann@arm.com>,
	<rafael@kernel.org>, <viresh.kumar@linaro.org>, <xuwei5@hisilicon.com>,
	<zhanjie9@hisilicon.com>
CC: <sudeep.holla@arm.com>, <cristian.marussi@arm.com>,
	<linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
	<quic_rgottimu@quicinc.com>, <linux-arm-kernel@lists.infradead.org>,
	<linux-pm@vger.kernel.org>, <d-gole@ti.com>
References: <20240312103723.3469762-1-quic_sibis@quicinc.com>
Content-Language: en-US
In-Reply-To: <20240312103723.3469762-1-quic_sibis@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

Oh, I just catch this issue too recently.

Also test this patch on my board and it's works fine to me.

Tested-by:Yipeng Zou <zouyipeng@huawei.com> <mailto:zouyipeng@huawei.com>

Reviewed-by: Yipeng Zou <zouyipeng@huawei.com> <mailto:zouyipeng@huawei.com>

在 2024/3/12 18:37, Sibi Sankar 写道:
> In the existing code, per-policy flags doesn't have any impact i.e.
> if cpufreq_driver boost is enabled and one or more of the per-policy
> boost is disabled, the cpufreq driver will behave as if boost is
> enabled. Fix this by incorporating per-policy boost flag in the policy->max
> calculus used in cpufreq_frequency_table_cpuinfo and setting the default
> per-policy boost to mirror the cpufreq_driver boost flag.
>
> Fixes: 218a06a79d9a ("cpufreq: Support per-policy performance boost")
> Reported-by: Dietmar Eggemann<dietmar.eggemann@arm.com>
> Reviewed-by: Viresh Kumar<viresh.kumar@linaro.org>
> Reviewed-by: Dhruva Gole<d-gole@ti.com>
> Signed-off-by: Sibi Sankar<quic_sibis@quicinc.com>
> ---
>
> v3:
> * Pickup Rbs.
> * Simplify per-policy boost setting. [Viresh]
>
> v2:
> * Enable per-policy boost flag in the core instead. [Viresh]
> * Add more details regarding the bug. [Viresh]
> * Drop cover-letter and patch 2.
>
> Logs reported-by Dietmar Eggemann:
> https://lore.kernel.org/lkml/265e5f2c-9b45-420f-89b1-44369aeb8418@arm.com/
>
>   drivers/cpufreq/cpufreq.c    | 18 ++++++++++++------
>   drivers/cpufreq/freq_table.c |  2 +-
>   2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index f6f8d7f450e7..66e10a19d76a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -653,14 +653,16 @@ static ssize_t store_local_boost(struct cpufreq_policy *policy,
>   	if (policy->boost_enabled == enable)
>   		return count;
>   
> +	policy->boost_enabled = enable;
> +
>   	cpus_read_lock();
>   	ret = cpufreq_driver->set_boost(policy, enable);
>   	cpus_read_unlock();
>   
> -	if (ret)
> +	if (ret) {
> +		policy->boost_enabled = !policy->boost_enabled;
>   		return ret;
> -
> -	policy->boost_enabled = enable;
> +	}
>   
>   	return count;
>   }
> @@ -1428,6 +1430,9 @@ static int cpufreq_online(unsigned int cpu)
>   			goto out_free_policy;
>   		}
>   
> +		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +		policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> +
>   		/*
>   		 * The initialization has succeeded and the policy is online.
>   		 * If there is a problem with its frequency table, take it
> @@ -2769,11 +2774,12 @@ int cpufreq_boost_trigger_state(int state)
>   
>   	cpus_read_lock();
>   	for_each_active_policy(policy) {
> +		policy->boost_enabled = state;
>   		ret = cpufreq_driver->set_boost(policy, state);
> -		if (ret)
> +		if (ret) {
> +			policy->boost_enabled = !policy->boost_enabled;
>   			goto err_reset_state;
> -
> -		policy->boost_enabled = state;
> +		}
>   	}
>   	cpus_read_unlock();
>   
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index c4d4643b6ca6..c17dc51a5a02 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -40,7 +40,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>   	cpufreq_for_each_valid_entry(pos, table) {
>   		freq = pos->frequency;
>   
> -		if (!cpufreq_boost_enabled()
> +		if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
>   		    && (pos->flags & CPUFREQ_BOOST_FREQ))
>   			continue;
>   

-- 
Regards,
Yipeng Zou