Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 28 May 2021 10:34:26 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Vincent Donnefort <vincent.donnefort@arm.com>
Cc:     peterz@infradead.org, rjw@rjwysocki.net,
        vincent.guittot@linaro.org, qperret@google.com,
        linux-kernel@vger.kernel.org, ionela.voinescu@arm.com,
        lukasz.luba@arm.com, dietmar.eggemann@arm.com
Subject: Re: [PATCH v2 3/3] PM / EM: Skip inefficient OPPs
Message-ID: <20210528050426.txs4kx2s37jwzpfe@vireshk-i7>
References: <1621616064-340235-1-git-send-email-vincent.donnefort@arm.com>
 <1621616064-340235-4-git-send-email-vincent.donnefort@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1621616064-340235-4-git-send-email-vincent.donnefort@arm.com>
User-Agent: NeoMutt/20180716-391-311a52
Precedence: bulk

On 21-05-21, 17:54, Vincent Donnefort wrote:
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> +static inline
> +struct em_perf_state *em_pd_get_efficient_state(struct em_perf_domain *pd,
> +						unsigned long freq)
> +{
> +	struct em_perf_state *ps;
> +	int i;
> +
> +	for (i = 0; i < pd->nr_perf_states; i++) {
> +		ps = &pd->table[i];
> +		if (ps->flags & EM_PERF_STATE_INEFFICIENT)
> +			continue;
> +		if (ps->frequency >= freq)
> +			break;

I believe it may be more optimal if we change the sequence of these two 'if'
blocks here. We only need to check for inefficient frequencies if it is >= freq.

> +	}
> +
> +	return ps;
> +}


> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> @@ -153,6 +154,9 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
>  
>  	freq = map_util_freq(util, freq, max);
>  
> +	/* Avoid inefficient performance states */
> +	freq = em_pd_get_efficient_freq(em_cpu_get(policy->cpu), freq);
> +
>  	if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
>  		return sg_policy->next_freq;

Assume this freq-table (E=efficient, IE=inefficient): 800M (E), 1G (E), 1.2G (IE), 1.4G (IE), 1.6G (E).
Thermal limits max to 1.4G

Freq returned by map_util_freq() is 1.01G.

Will we not end up selecting 1.4G here ? Inefficient as well as much higher than
what we requested for ?

-- 
viresh