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:   Wed, 26 May 2021 09:17:51 +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 0/3] EM / PM: Inefficient OPPs
Message-ID: <20210526034751.5fl4kekq73gqy2wq@vireshk-i7>
References: <1621616064-340235-1-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-1-git-send-email-vincent.donnefort@arm.com>
User-Agent: NeoMutt/20180716-391-311a52
Precedence: bulk

On 21-05-21, 17:54, Vincent Donnefort wrote:
> We (Power team in Arm) are working with an experimental kernel for the
> Google's Pixel4 to evaluate and improve the current mainline performance
> and energy consumption on a real life device with Android.
> 
> The SD855 SoC found in this phone has several OPPs that are inefficient.
> I.e. despite a lower frequency, they have a greater cost. (That cost being 
> fmax * OPP power / OPP freq). This issue is twofold. First of course,
> running a specific workload at an inefficient OPP is counterproductive
> since it wastes wasting energy. But also, inefficient OPPs make a
> performance domain less appealing for task placement than it really is.
> 
> We evaluated the change presented here by running 30 iterations of Android 
> PCMark "Work 2.0 Performance". While we did not see any statistically
> significant performance impact, this change allowed to drastically improve 
> the idle time residency.   
>  
> 
>                            |   Running   |  WFI [1]  |    Idle   |
>    ------------------------+-------------+-----------+-----------+
>    Little cluster (4 CPUs) |    -0.35%   |   +0.35%  |   +0.79%  |
>    ------------------------+-------------+-----------+-----------+
>    Medium cluster (3 CPUs) |    -6.3%    |    -18%   |    +12%   |
>    ------------------------+-------------+-----------+-----------+
>    Big cluster    (1 CPU)  |    -6.4%    |    -6.5%  |    +2.8%  |
>    ------------------------+-------------+-----------+-----------+
> 
> On the SD855, the inefficient OPPs are found on the little cluster. By
> removing them from the Energy Model, we make the most efficient CPUs more
> appealing for task placement, helping to reduce the running time for the
> medium and big CPUs. Increasing idle time is crucial for this platform due 
> to the substantial energy cost differences among the clusters. Also,
> despite not appearing in the statistics (the idle driver used here doesn't 
> report it), we can speculate that we also improve the cluster idle time.

First of all, sorry about not replying earlier. I have seen this earlier and was
shying away to receive some feedback from Rafael/Peter instead :(

I think the problem you mention is genuine, I have realized it in the past,
discussed with Vincent Guittot (cc'd) but never was able to get to a proper
solution as the EM model wasn't there then.

I have seen your approach (from top level) and I feel maybe we can improve upon
the whole idea a bit, lemme know what you think. The problem I see with this
approach is the unnecessary updates to schedutil that this series makes, which
IMHO is the wrong thing to do. Schedutil isn't the only governor and such
changes will end up making the performance delta between ondemand and schedutil
even more (difference based on their core design philosophy is fine, but these
are improvements which each of them should enjoy). And if another governor wants
these smart decisions to be added there, then it is trouble again.

Since the whole thing depends on EM and OPPs, I think we can actually do this.

When the cpufreq driver registers with the EM core, lets find all the
Inefficient OPPs and disable them once and for all. Of course, this must be done
on voluntarily basis, a flag from the drivers will do. With this, we won't be
required to update any thing at any of the governors end.

Will that work ?

-- 
viresh