Re: [PATCH V2] opp: Remove bandwidth votes when target_freq is zero

From: Sibi Sankar <sibis@codeaurora.org>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-pm@vger.kernel.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	georgi.djakov@linaro.org, linux-kernel@vger.kernel.org,
	linux-kernel-owner@vger.kernel.org
Subject: Re: [PATCH V2] opp: Remove bandwidth votes when target_freq is zero
Date: Thu, 28 May 2020 00:01:19 +0530	[thread overview]
Message-ID: <8bcec7186a4fc0d291d834a5964dab2f@codeaurora.org> (raw)
In-Reply-To: <3aa3870d71b536127bb6af88c1dbfb4672ba4173.1590552778.git.viresh.kumar@linaro.org>

On 2020-05-27 09:43, Viresh Kumar wrote:
> We already drop several votes when target_freq is set to zero, drop
> bandwidth votes as well.
> 
> Reported-by: Sibi Sankar <sibis@codeaurora.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2: Some changes left uncommited in my tree by mistake.

Viresh,
Thanks for the patch :)

Tested-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>

> 
>  drivers/opp/core.c | 49 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 37 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 56d3022c1ca2..df12c3804533 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -725,6 +725,34 @@ static int _generic_set_opp_regulator(struct
> opp_table *opp_table,
>  	return ret;
>  }
> 
> +static int _set_opp_bw(const struct opp_table *opp_table,
> +		       struct dev_pm_opp *opp, struct device *dev, bool remove)
> +{
> +	u32 avg, peak;
> +	int i, ret;
> +
> +	if (!opp_table->paths)
> +		return 0;
> +
> +	for (i = 0; i < opp_table->path_count; i++) {
> +		if (remove) {
> +			avg = 0;
> +			peak = 0;
> +		} else {
> +			avg = opp->bandwidth[i].avg;
> +			peak = opp->bandwidth[i].peak;
> +		}
> +		ret = icc_set_bw(opp_table->paths[i], avg, peak);
> +		if (ret) {
> +			dev_err(dev, "Failed to %s bandwidth[%d]: %d\n",
> +				remove ? "remove" : "set", i, ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int _set_opp_custom(const struct opp_table *opp_table,
>  			   struct device *dev, unsigned long old_freq,
>  			   unsigned long freq,
> @@ -820,7 +848,7 @@ int dev_pm_opp_set_rate(struct device *dev,
> unsigned long target_freq)
>  	unsigned long freq, old_freq, temp_freq;
>  	struct dev_pm_opp *old_opp, *opp;
>  	struct clk *clk;
> -	int ret, i;
> +	int ret;
> 
>  	opp_table = _find_opp_table(dev);
>  	if (IS_ERR(opp_table)) {
> @@ -837,12 +865,17 @@ int dev_pm_opp_set_rate(struct device *dev,
> unsigned long target_freq)
>  		if (!_get_opp_count(opp_table))
>  			return 0;
> 
> -		if (!opp_table->required_opp_tables && !opp_table->regulators) {
> +		if (!opp_table->required_opp_tables && !opp_table->regulators &&
> +		    !opp_table->paths) {
>  			dev_err(dev, "target frequency can't be 0\n");
>  			ret = -EINVAL;
>  			goto put_opp_table;
>  		}
> 
> +		ret = _set_opp_bw(opp_table, opp, dev, true);
> +		if (ret)
> +			return ret;
> +
>  		if (opp_table->regulator_enabled) {
>  			regulator_disable(opp_table->regulators[0]);
>  			opp_table->regulator_enabled = false;
> @@ -932,16 +965,8 @@ int dev_pm_opp_set_rate(struct device *dev,
> unsigned long target_freq)
>  			dev_err(dev, "Failed to set required opps: %d\n", ret);
>  	}
> 
> -	if (!ret && opp_table->paths) {
> -		for (i = 0; i < opp_table->path_count; i++) {
> -			ret = icc_set_bw(opp_table->paths[i],
> -					 opp->bandwidth[i].avg,
> -					 opp->bandwidth[i].peak);
> -			if (ret)
> -				dev_err(dev, "Failed to set bandwidth[%d]: %d\n",
> -					i, ret);
> -		}
> -	}
> +	if (!ret)
> +		ret = _set_opp_bw(opp_table, opp, dev, false);
> 
>  put_opp:
>  	dev_pm_opp_put(opp);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.