Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Saravana Kannan <saravanak@google.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Android Kernel Team <kernel-team@android.com>,
	Linux PM <linux-pm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
Date: Thu, 11 Jul 2019 16:16:46 -0700
Message-ID: <CAGETcx_AiziadEDBrTiXa0qy5dMp3MM=8JLkuESg_SJ4sLZzqQ@mail.gmail.com> (raw)
In-Reply-To: <CAGETcx-gSQLtbM+7WgqNvhCypmwoMNjkAkGDqRnK=PUGUOxjkQ@mail.gmail.com>

Bump. I saw your email in Sibi's patch series. His patch series is
just one use case/user of this feature. This patch series is useful in
general. Do plan to Ack this? Or thoughts on my earlier response?

Thanks,
Saravana

On Fri, Jun 28, 2019 at 1:26 PM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Jun 27, 2019 at 11:49 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 26-06-19, 11:10, Saravana Kannan wrote:
> > > On Tue, Jun 25, 2019 at 11:32 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > > > So, when a CPU changes frequency, we must change the performance state
> > > > of PM domain and change frequency/bw of the cache synchronously.
> > >
> > > I mean, it's going to be changed when we get the CPUfreq transition
> > > notifiers. From a correctness point of view, setting it inside the OPP
> > > framework is not any better than doing it when we get the notifiers.
> >
> > That's what the problem is. All maintainers now a days ask people to
> > stay away from notifiers and we are making that base of another new
> > thing we are starting.
>
> In that case we can just add direct calls in cpufreq.c to let devfreq
> know about the frequency changes. But then again, CPU is just one
> example for this use case. I'm just using that because people are more
> familiar with that.
>
> > Over that, with many cpufreq drivers we have fast switching enabled
> > and notifiers disabled. How will they make these things work ? We
> > still want to scale L3 in those cases as well.
>
> Nothing is preventing them from using the xlate OPP API I added to
> figure out all the CPU to L3 frequency mapping and then set the L3
> frequency directly from the CPUfreq driver.
>
> Also, whether we use OPP framework or devfreq to set the L3 frequency,
> it's going to block fast switching because both these frameworks have
> APIs that can sleep.
>
> But really, most mobile use cases don't want to permanently tie L3
> freq to CPU freq. Having it go through devfreq and being able to
> switch governors is a very important need for mobile products.
>
> Keep in mind that nothing in this series does any of the cpufreq stuff
> yet. That'll need a few more changes. I was just using CPUfreq as an
> example.
>
> > > I see this as "required for good performance". So I don't see it as
> > > redefining required-opps. If someone wants good performance/power
> > > balance they follow the "required-opps". Technically even the PM
> > > pstates are required for good power. Otherwise, the system could leave
> > > the voltage at max and stuff would still work.
> > >
> > > Also, the slave device might need to get input from multiple master
> > > devices and aggregate the request before setting the slave device
> > > frequency. So I don't think OPP  framework would be the right place to
> > > deal with those things. For example, L3 might (will) have different
> > > mappings for big vs little cores. So that needs to be aggregated and
> > > set properly by the slave device driver. Also, GPU might have a
> > > mapping for L3 too. In which case the L3 slave driver needs to take
> > > input from even more masters before it decides its frequency. But most
> > > importantly, we still need the ability to change governors for L3.
> > > Again these are just examples with L3 and it can get more complicated
> > > based on the situation.
> > >
> > > Most importantly, instead of always going by mapping, one might decide
> > > to scale the L3 based on some other governor (that looks at some HW
> > > counter). Or just set it to performance governor for a use case for
> > > which performance is more important. All of this comes for free with
> > > devfreq and if we always set it from OPP framework we don't give this
> > > required control to userspace.
> > >
> > > I think going through devfreq is the right approach for this. And we
> > > can always rewrite the software if we find problems in the future. But
> > > as it stands today, this will help cases like exynos without the need
> > > for a lot of changes. Hope I've convinced you.
> >
> > I understand the aggregation thing and fully support that the
> > aggregation can't happen in OPP core and must be done somewhere else.
> > But the input can go from OPP core while the frequency is changing,
> > isn't it ?
>
> I'm not opposed to OPP sending input to devfreq to let it know that a
> master device frequency change is happening. But I think this is kinda
> orthogonal to this patch series.
>
> Today the passive governor looks at the master device's devfreq
> frequency changes to trigger the frequency change of the slave
> devfreq. It neither supports tracking OPP frequency change nor CPUfreq
> frequency change. If that's something we want to add, we can look into
> that separately as passive governor (or a new governor) changes.
>
> But then not all devices (CPUfreq or otherwise) use OPP to set the
> frequencies. So it's beneficial to have all of these frameworks as
> inputs for devfreq passive (like) governor. CPUfreq is actually a bit
> more tricky because we'll also have to track hotplug, etc. So direct
> calls from CPUfreq to devfreq (similar to cpufreq stats tracking)
> would be good.
>
> -Saravana

      reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-22  0:34 Saravana Kannan
2019-06-22  0:34 ` [PATCH v1 1/3] OPP: Allow required-opps even if the device doesn't have power-domains Saravana Kannan
2019-06-22  0:34 ` [PATCH v1 2/3] OPP: Add function to look up required OPP's for a given OPP Saravana Kannan
2019-06-22 11:49   ` Chanwoo Choi
2019-06-22 21:41     ` Saravana Kannan
2019-06-23  4:27       ` Chanwoo Choi
2019-06-23  6:07         ` Saravana Kannan
2019-06-22  0:34 ` [PATCH v1 3/3] PM / devfreq: Add required OPPs support to passive governor Saravana Kannan
2019-06-22 12:00   ` Chanwoo Choi
2019-06-22 21:45     ` Saravana Kannan
2019-06-24  9:43 ` [PATCH v1 0/3] Add required-opps support to devfreq passive gov Viresh Kumar
2019-06-24 22:17   ` Saravana Kannan
2019-06-25  4:10     ` Viresh Kumar
2019-06-25  5:00       ` Saravana Kannan
2019-06-25  5:22         ` Viresh Kumar
2019-06-25  5:29           ` Saravana Kannan
2019-06-26  6:32             ` Viresh Kumar
2019-06-26 18:10               ` Saravana Kannan
2019-06-28  6:49                 ` Viresh Kumar
2019-06-28 20:26                   ` Saravana Kannan
2019-07-11 23:16                     ` Saravana Kannan [this message]

Reply instructions:

You may reply publically 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='CAGETcx_AiziadEDBrTiXa0qy5dMp3MM=8JLkuESg_SJ4sLZzqQ@mail.gmail.com' \
    --to=saravanak@google.com \
    --cc=cw00.choi@samsung.com \
    --cc=kernel-team@android.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@kernel.org \
    --cc=viresh.kumar@linaro.org \
    --cc=vireshk@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 linux-pm@archiver.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