Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Ionela Voinescu <ionela.voinescu@arm.com>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Will Deacon <will@kernel.org>,
	Russell King - ARM Linux <linux@armlinux.org.uk>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/8] cpufreq: allow drivers to flag custom support for freq invariance
Date: Thu, 9 Jul 2020 09:53:54 +0100
Message-ID: <20200709085354.GA5623@arm.com> (raw)
In-Reply-To: <389dd87f-fed0-e4ea-81f3-5491fd2a54d1@arm.com>

Hi guys,

On Monday 06 Jul 2020 at 14:14:47 (+0200), Dietmar Eggemann wrote:
> On 02/07/2020 13:44, Ionela Voinescu wrote:
> > Hi,
> > 
> > On Thursday 02 Jul 2020 at 08:28:18 (+0530), Viresh Kumar wrote:
> >> On 01-07-20, 18:05, Rafael J. Wysocki wrote:
> >>> On Wed, Jul 1, 2020 at 3:33 PM Ionela Voinescu <ionela.voinescu@arm.com> wrote:
> >>>> On Wednesday 01 Jul 2020 at 16:16:17 (+0530), Viresh Kumar wrote:
> 
> [...]
> 
> >> There can be other reasons which we aren't able to imagine at this
> >> point of time.
> >>
> > 
> > But I understand both the points you and Rafael raised so it's obvious
> > that a 'opt in' flag would be the better option.
> 
> Why can't we just move the arch_set_freq_scale() call from cpufreq
> driver to cpufreq core w/o introducing a FIE related driver flag?
> 
> Current scenario for Frequency Invariance Engine (FIE) on arm/arm64.
> 
> +------------------------------+       +------------------------------+
> |                              |       |                              |
> | cpufreq core:                |       | arch: (arm, arm64)           |
> 
> |                              |       |                              |
> | weak arch_set_freq_scale() {}|       |                              |
> |                              |       |                              |
> +------------------------------+       |                              |
>                                        |                              |
> +------------------------------+       |                              |
> |                              |       |                              |
> | cpufreq driver:              |       |                              |
> |                            +-----------> arch_set_freq_scale()      |
> |                              |       |   {                          |
> +------------------------------+       |      if (use counters)       |
>                                        |        return;               |
> +------------------------------+       |      ...                     |
> |                              |       |   }                          |
> | task scheduler:              |       |                              |
> |                            +-----------> arch_scale_freq_tick()*    |
> |                              |       |   {                          |
> 
> |                              |       |      if (!use counters)      |
> |                              |       |        return;               |
> |                              |       |      ...                     |
> |                              |       |   }                          |
> +------------------------------+       +------------------------------+
> 
> * defined as topology_scale_freq_tick() in arm64
> 
> Only Arm/Arm64 defines arch_set_freq_scale() to get the 'legacy' CPUfreq
> based FIE. This would still be the case when we move
> arch_set_freq_scale() from individual cpufreq drivers to cpufreq core.
> 
> Arm64 is the only arch which has to runtime-choose between two different
> FIEs. This is currently done by bailing out early in one of the FIE
> functions based on 'use counters'.
> 
> X86 (and others) will continue to not define arch_set_freq_scale().
> 
> The issue with CONFIG_BL_SWITCHER (vexpress-spc-cpufreq.c) could be
> solved arm/arm64 internally (arch_topology.c) by putting
> arch_set_freq_scale() under a !CONFIG_BL_SWITCHER guard.
> I doubt that there are any arm bL systems out there running it. At least
> I'm not aware of any complaints due to missing FIE support in bl
> switcher setups so far.

Thank you Dietmar, for your review.

I was trying to suggest the same in my other replies. Given that BL_SWITCHER
can be removed as an argument for introducing a flag, I would also find it
cleaner to just skip on introducing a flag altogether, at least until we
have a driver/scenario in the kernel that will functionally benefit from it.
This would also give us the chance to reconsider the best meaning of the
flag we later introduce.

The introduction of the 'opt in' flag would be the next best thing as
suggested in the other replies, but currently it would not result in
anything functionally different.


Rafael, Viresh, would you mind confirming whether you still consider
having an 'opt in' flag is preferable here?

Many thanks,
Ionela.

  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-01  9:07 [PATCH 0/8] cpufreq: improve frequency invariance support Ionela Voinescu
2020-07-01  9:07 ` [PATCH 1/8] cpufreq: allow drivers to flag custom support for freq invariance Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01 13:33     ` Ionela Voinescu
2020-07-01 16:05       ` Rafael J. Wysocki
2020-07-01 18:06         ` Ionela Voinescu
2020-07-02  2:58         ` Viresh Kumar
2020-07-02 11:44           ` Ionela Voinescu
2020-07-06 12:14             ` Dietmar Eggemann
2020-07-09  8:53               ` Ionela Voinescu [this message]
2020-07-09  9:09                 ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 2/8] cpufreq: move invariance setter calls in cpufreq core Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01 15:27     ` Ionela Voinescu
2020-07-01 15:51       ` Rafael J. Wysocki
2020-07-02  3:01         ` Viresh Kumar
2020-07-02 11:45         ` Ionela Voinescu
2020-07-01  9:07 ` [PATCH 3/8] cpufreq,drivers: remove setting of frequency scale factor Ionela Voinescu
2020-07-01  9:07 ` [PATCH 4/8] cpufreq,vexpress-spc: fix Frequency Invariance (FI) for bL switching Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01 14:07     ` Ionela Voinescu
2020-07-02  3:05       ` Viresh Kumar
2020-07-02 11:41         ` Ionela Voinescu
2020-07-02 11:46           ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 5/8] cpufreq: report whether cpufreq supports Frequency Invariance (FI) Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 6/8] arch_topology,cpufreq,sched/core: constify arch_* cpumasks Ionela Voinescu
2020-07-01  9:07 ` [PATCH 7/8] arch_topology,arm64: define arch_scale_freq_invariant() Ionela Voinescu
2020-07-01  9:07 ` [PATCH 8/8] cpufreq: make schedutil the default for arm and arm64 Ionela Voinescu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200709085354.GA5623@arm.com \
    --to=ionela.voinescu@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sudeep.holla@arm.com \
    --cc=valentin.schneider@arm.com \
    --cc=viresh.kumar@linaro.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git