LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Saravana Kannan <saravanak@google.com>
To: Saravana Kannan <saravanak@google.com>,
	georgi.djakov@linaro.org, amit.kucheria@linaro.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	daidavid1@codeaurora.org, devicetree@vger.kernel.org,
	evgreen@chromium.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	nm@ti.com, rjw@rjwysocki.net, Rob Herring <robh+dt@kernel.org>,
	sboyd@kernel.org, seansw@qti.qualcomm.com, sibis@codeaurora.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	vireshk@kernel.org, Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH v2 0/5] Introduce OPP bandwidth bindings
Date: Mon, 3 Jun 2019 12:12:30 -0700
Message-ID: <CAGETcx8yV_D+=qLnJOx5s5Nvq2RxhcJvz+gejDBN1-qrBE=Msg@mail.gmail.com> (raw)
In-Reply-To: <20190603155634.GA10741@jcrouse1-lnx.qualcomm.com>

On Mon, Jun 3, 2019 at 8:56 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> On Fri, May 31, 2019 at 07:12:28PM -0700, Saravana Kannan wrote:
> > I'll have to Nack this series because it's making a couple of wrong assumptions
> > about bandwidth voting.
> >
> > Firstly, it's mixing up OPP to bandwidth mapping (Eg: CPU freq to CPU<->DDR
> > bandwidth mapping) with the bandwidth levels that are actually supported by an
> > interconnect path (Eg: CPU<->DDR bandwidth levels). For example, CPU0 might
> > decide to vote for a max of 10 GB/s because it's a little CPU and never needs
> > anything higher than 10 GB/s even at CPU0's max frequency. But that has no
> > bearing on bandwidth level available between CPU<->DDR.
>
> I'm going to just quote this part of the email to avoid forcing people to
> scroll too much.
>
> I agree that there is an enormous universe of new and innovative things that can
> be done for bandwidth voting. I would love to have smart governors and expansive
> connections between different components that are all aware of each other. I
> don't think that anybody is discounting that these things are possible.
>
> But as it stands today, as a leaf driver developer my primary concern is that I
> need to vote something for the GPU->DDR path. Right now I'm voting the maximum
> because that is the bare minimum we need to get working GPU.
>
> Then the next incremental baby step is to allow us to select a minimum
> vote based on a GPU frequency level to allow for some sort of very coarse power
> savings. It isn't perfect, but better than cranking everything to 11.

I completely agree. I'm not saying you shouldn't do bandwidth voting
based on device frequency. In some cases, it's actually the right
thing to do too.

> This is
> why we need the OPP bandwidth bindings to allow us to make the association and
> tune down the vote.

Again, I'm perfectly fine with this too.

> I fully agree that this isn't the optimal solution but
> it is the only knob we have right now.
> And after that we should go nuts. I'll gladly put the OPP bindings in the
> rear-view mirror and turn over all bandwidth to a governor or two or three.

This is the problem part in the series. Once a property is exposed in
DT, we can't just take it back. A new kernel needs to continue
supporting old compiled DT binaries. So if we know we'll have to
change a DT property in the future to be "more correct", then we
should just do that one instead of "for now" bindings.

And I even proposed what the new bindings should look like and why we
should do it that way.

I'll try to get some patches out for that in the near future. But
doesn't have to be just from me. I'm just pointing out why the current
bindings aren't good/scalable.

> I'll be happy to have nothing to do with it again. But until then we need
> a solution for the leaf drivers that lets us provide some modicum of power
> control.

Agreed.

-Saravana

      reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-23 13:28 Georgi Djakov
2019-04-23 13:28 ` [PATCH v2 1/5] dt-bindings: opp: Introduce bandwidth-MBps bindings Georgi Djakov
2019-04-24  5:33   ` Viresh Kumar
2019-04-24  6:46   ` Rajendra Nayak
2019-04-24  6:49     ` Viresh Kumar
2019-04-24  9:00       ` Sibi Sankar
2019-04-24  9:05         ` Viresh Kumar
2019-04-25  4:24         ` Bjorn Andersson
2019-04-24  8:44   ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 2/5] interconnect: Add of_icc_get_by_index() helper function Georgi Djakov
2019-05-07 11:59   ` Sibi Sankar
2019-06-27  5:56     ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 3/5] OPP: Add support for parsing the interconnect bandwidth Georgi Djakov
2019-04-24  5:52   ` Viresh Kumar
2019-06-27  6:27     ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 4/5] OPP: Update the bandwidth on OPP frequency changes Georgi Djakov
2019-04-24  5:55   ` Viresh Kumar
2019-04-24 10:05   ` Sibi Sankar
2019-04-23 13:28 ` [PATCH v2 5/5] cpufreq: dt: Add support for interconnect bandwidth scaling Georgi Djakov
2019-06-01  2:12 ` [PATCH v2 0/5] Introduce OPP bandwidth bindings Saravana Kannan
2019-06-03 15:56   ` Jordan Crouse
2019-06-03 19:12     ` 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='CAGETcx8yV_D+=qLnJOx5s5Nvq2RxhcJvz+gejDBN1-qrBE=Msg@mail.gmail.com' \
    --to=saravanak@google.com \
    --cc=amit.kucheria@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daidavid1@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=evgreen@chromium.org \
    --cc=georgi.djakov@linaro.org \
    --cc=kernel-team@android.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=seansw@qti.qualcomm.com \
    --cc=sibis@codeaurora.org \
    --cc=vincent.guittot@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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

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


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


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