Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: David Dai <davidai@google.com>
Cc: Saravana Kannan <saravanak@google.com>,
	Rob Herring <robh@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Quentin Perret <qperret@google.com>,
	Masami Hiramatsu <mhiramat@google.com>,
	Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Marc Zyngier <maz@kernel.org>,
	Oliver Upton <oliver.upton@linux.dev>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Pavan Kondeti <quic_pkondeti@quicinc.com>,
	Gupta Pankaj <pankaj.gupta@amd.com>, Mel Gorman <mgorman@suse.de>,
	kernel-team@android.com, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/2] dt-bindings: cpufreq: add virtual cpufreq device
Date: Tue, 7 May 2024 11:21:53 +0100	[thread overview]
Message-ID: <ZjoAwVKvyHzX4_QW@bogus> (raw)
In-Reply-To: <CABN1KCLbhh9Rf9R2J2UoTS+6Dzc8yysOedKgXizPbQvYuG8tqQ@mail.gmail.com>

On Thu, May 02, 2024 at 01:17:57PM -0700, David Dai wrote:
> On Thu, Feb 15, 2024 at 3:26 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Fri, Feb 02, 2024 at 09:53:52AM -0600, Rob Herring wrote:
> > > On Wed, Jan 31, 2024 at 10:23:03AM -0800, Saravana Kannan wrote:
> > > >
> > > > We also need the OPP tables to indicate which CPUs are part of the
> > > > same cluster, etc. Don't want to invent a new "protocol" and just use
> > > > existing DT bindings.
> > >
> > > Topology binding is for that.
> > >
> > > What about when x86 and other ACPI systems need to do this too? You
> > > define a discoverable interface, then it works regardless of firmware.
> > > KVM, Virtio, VFIO, etc. are all their own protocols.
> > >
> >
> > +1 for the above. I have mentioned the same couple of times but I am told
> > it can be taken up later which I fail to understand. Once we define DT
> > bindings, it must be supported for long time which doesn't provide any
> > motivation to such a discoverable interface which works on any virtual
> > platforms irrespective of the firmware.
> >
> 
> Hi Sudeep,
> 
> We are thinking of a discoverable interface like this, where the
> performance info and performance domain mappings are discoverable
> through the device registers. This should make it more portable across
> firmwares. Would this address your concerns?

Yes.

> Also, you asked to  document this.
> Where exactly would you want to document this?

IMO it could go under Documentation/firmware-guide ? Unless someone
has any other suggestions.

> AFAIK the DT bindings documentation is not supposed to include this level of
> detail. Would a comment in the driver be sufficient?

Agree, DT bindings is not the right place. May be even comment in the
driver would be sufficient.

Overall it looks good and on the right path IMO.

> 
> CPU0..CPUn
> +-------------+-------------------------------+--------+-------+
> | Register    | Description                   | Offset |   Len |
> +-------------+-------------------------------+--------+-------+
> | cur_perf    | read this register to get     |    0x0 |   0x4 |
> |             | the current perf (integer val |        |       |
> |             | representing perf relative to |        |       |
> |             | max performance)              |        |       |
> |             | that vCPU is running at       |        |       |
> +-------------+-------------------------------+--------+-------+
> | set_perf    | write to this register to set |    0x4 |   0x4 |
> |             | perf value of the vCPU        |        |       |
> +-------------+-------------------------------+--------+-------+
> | perftbl_len | number of entries in perf     |    0x8 |   0x4 |
> |             | table. A single entry in the  |        |       |
> |             | perf table denotes no table   |        |       |
> |             | and the entry contains        |        |       |
> |             | the maximum perf value        |        |       |
> |             | that this vCPU supports.      |        |       |
> |             | The guest can request any     |        |       |
> |             | value between 1 and max perf. |        |       |

Does this have to be per cpu ? It can be simplified by keeping
just cur_perf, set_perf and perf_domain in per-cpu entries and this
per domain entries separate. But I am not against per cpu entries
as well.

Also why do you need the table if the guest can request any value from
1 to max perf ? The table will have discrete OPPs ? If so, how to they
map to the perf range [1 - maxperf] ?

> +---------------------------------------------+--------+-------+
> | perftbl_sel | write to this register to     |    0xc |   0x4 |
> |             | select perf table entry to    |        |       |
> |             | read from                     |        |       |
> +---------------------------------------------+--------+-------+
> | perftbl_rd  | read this register to get     |   0x10 |   0x4 |
> |             | perf value of the selected    |        |       |
> |             | entry based on perftbl_sel    |        |       |
> +---------------------------------------------+--------+-------+
> | perf_domain | performance domain number     |   0x14 |   0x4 |
> |             | that this vCPU belongs to.    |        |       |
> |             | vCPUs sharing the same perf   |        |       |
> |             | domain number are part of the |        |       |
> |             | same performance domain.      |        |       |
> +-------------+-------------------------------+--------+-------+

The above are couple of high level questions I have ATM.

--
Regards,
Sudeep

  reply	other threads:[~2024-05-07 10:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-27  0:43 [PATCH v5 0/2] Improve VM CPUfreq and task placement behavior David Dai
2024-01-27  0:43 ` [PATCH v5 1/2] dt-bindings: cpufreq: add virtual cpufreq device David Dai
2024-01-31 17:06   ` Rob Herring
2024-01-31 18:23     ` Saravana Kannan
2024-02-02 15:53       ` Rob Herring
2024-02-04 10:23         ` Marc Zyngier
2024-03-11 11:40           ` Quentin Perret
2024-02-05  8:38         ` Viresh Kumar
2024-02-05 16:39           ` Rob Herring
2024-02-15 11:26         ` Sudeep Holla
2024-05-02 20:17           ` David Dai
2024-05-07 10:21             ` Sudeep Holla [this message]
2024-05-17 20:59               ` David Dai
2024-01-27  0:43 ` [PATCH v5 2/2] cpufreq: add virtual-cpufreq driver David Dai
2024-01-31  1:13   ` kernel test robot

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=ZjoAwVKvyHzX4_QW@bogus \
    --to=sudeep.holla@arm.com \
    --cc=conor+dt@kernel.org \
    --cc=davidai@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=kernel-team@android.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=mgorman@suse.de \
    --cc=mhiramat@google.com \
    --cc=oliver.upton@linux.dev \
    --cc=pankaj.gupta@amd.com \
    --cc=peterz@infradead.org \
    --cc=qperret@google.com \
    --cc=quic_pkondeti@quicinc.com \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=saravanak@google.com \
    --cc=vincent.guittot@linaro.org \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).