Re: [PATCH v3 3/6] cpufreq: Add an interface to mark inefficient frequencies

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Vincent Donnefort <vincent.donnefort@arm.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Lukasz Luba <lukasz.luba@arm.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Quentin Perret <qperret@google.com>,
	Linux PM <linux-pm@vger.kernel.org>,
	Ionela Voinescu <ionela.voinescu@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>
Subject: Re: [PATCH v3 3/6] cpufreq: Add an interface to mark inefficient frequencies
Date: Tue, 6 Jul 2021 14:11:24 +0200	[thread overview]
Message-ID: <CAJZ5v0h+zZZOn70eGD9XZy0rcy0ic-C1=UC7YGLhUahVgC4pzg@mail.gmail.com> (raw)
In-Reply-To: <20210706081256.GA216826@e120877-lin.cambridge.arm.com>

On Tue, Jul 6, 2021 at 10:13 AM Vincent Donnefort
<vincent.donnefort@arm.com> wrote:
>
> [...]
>
> > >
> > > What about a cpufreq_policy option that if sets would make
> > > cpufreq_frequency_table_target() skip inefficient OPPs while staying within
> > > the limit of max policy?
> >
> > That would work too, ->
> >
> > > Each governor could decide to set it or not, but
> > > it would hide the efficiency resolution to the governor and allow drivers
> > > that implements ->target() to also implements support for inefficient OPPs.
> >
> > -> but alternatively there could be an additional cpufreq driver flag
> > to be set by the drivers implementing ->target() and wanting to deal
> > with CPUFREQ_RELATION_EFFICIENT themselves (an opt-in of sorts).
> >
> > So the governors that want it may pass CPUFREQ_RELATION_EFFICIENT to
> > __cpufreq_driver_target() and then it will be passed to ->target()
> > depending on whether or not the new driver flag is set.
>
> Of course, I can implement this instead of a cpufreq_policy flag in v4.
> I suppose then right fallback for CPUFREQ_RELATION_EFFICIENT in case the
> driver doesn't opt-in is CPUFREQ_RELATION_L.
>
> >
> > > That flag could be set according to a new cpufreq_governor flag
> > > CPUFREQ_GOV_SKIP_INEFFICIENCIES?
> > >
> > > That could though modify behaviors like powersave_bias from ondemand. But if
> > > a frequency is inefficient, there's probably no power saving anyway.
> >
> > AFAICS, the userspace governor aside, using inefficient frequencies
> > only works with the powersave governor.  In the other cases,
> > RELATION_L (say) can be interpreted as "the closest efficient
> > frequency equal to or above the target" with the max policy limit
> > possibly causing inefficient frequencies to be used if they are closer
> > to the limit than the next efficient one.
> >
> > As a rule, the governors don't assume that there are any inefficient
> > frequencies in the table.  In fact, they don't make any assumptions
> > regarding the contents of the frequency table at all.  They don't even
> > assume that the driver uses a frequency table in the first place.
>
> So all the governors, beside powersave and userspace would replace their
> RELATION_L with RELATION_EFFICIENT. I'll add the changes in v4.
>
> So if I sum-up: new RELATION_EFFICIENT that resolves RELATION_L to an higher
> efficient frequency (if necessary) within the limits of policy->max.

Yes.

It can be called RELATION_E for brevity.

> CPUfreq drivers can opt-in by setting an appropriate flag. If they do not,
> RELATION_EFFICIENT will be rewritten in RELATION_L.

Yes, and cpufreq_frequency_table_target() will take RELATION_E into
account if set.

> All governors but userspace and powersave would use RELATION_EFFICIENT instead of RELATION_L.

Yes.

> If that works for you, I'll implement this in a v4, as well as some
> improvements for the CPUfreq/EM registration following the discussion with
> Viresh.

Sounds good, thanks!