Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Lina Iyer <ilina@codeaurora.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Raju P . L . S . S . S . N" <rplsssn@codeaurora.org>,
	Amit Kucheria <amit.kucheria@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Niklas Cassel <niklas.cassel@linaro.org>,
	Tony Lindgren <tony@atomide.com>,
	Kevin Hilman <khilman@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Souvik Chakravarty <souvik.chakravarty@arm.com>,
	linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Lina Iyer <lina.iyer@linaro.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>
Subject: Re: [PATCH 17/18] arm64: dts: Convert to the hierarchical CPU topology layout for MSM8916
Date: Wed, 17 Jul 2019 18:18:56 +0100
Message-ID: <20190717171856.GF18370@e107155-lin> (raw)
In-Reply-To: <20190716203631.GC25567@codeaurora.org>

On Tue, Jul 16, 2019 at 02:36:31PM -0600, Lina Iyer wrote:
> On Tue, Jul 16 2019 at 08:47 -0600, Sudeep Holla wrote:
> > On Mon, May 13, 2019 at 09:22:59PM +0200, Ulf Hansson wrote:
> > > From: Lina Iyer <lina.iyer@linaro.org>
> > >
> > > In the hierarchical layout, we are creating power domains around each CPU
> > > and describes the idle states for them inside the power domain provider
> > > node. Note that, the CPU's idle states still needs to be compatible with
> > > "arm,idle-state".
> > >
> > > Furthermore, represent the CPU cluster as a separate master power domain,
> > > powering the CPU's power domains. The cluster node, contains the idle
> > > states for the cluster and each idle state needs to be compatible with the
> > > "domain-idle-state".
> > >
> > > If the running platform is using a PSCI FW that supports the OS initiated
> > > CPU suspend mode, which likely should be the case unless the PSCI FW is
> > > very old, this change triggers the PSCI driver to enable it.
> > >
> > > Cc: Andy Gross <andy.gross@linaro.org>
> > > Cc: David Brown <david.brown@linaro.org>
> > > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> > > Co-developed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> >
> > [...]
> >
> > > @@ -166,12 +170,57 @@
> > >  				min-residency-us = <2000>;
> > >  				local-timer-stop;
> > >  			};
> > > +
> > > +			CLUSTER_RET: cluster-retention {
> > > +				compatible = "domain-idle-state";
> > > +				arm,psci-suspend-param = <0x1000010>;
> > > +				entry-latency-us = <500>;
> > > +				exit-latency-us = <500>;
> > > +				min-residency-us = <2000>;
> > > +			};
> > > +
> > > +			CLUSTER_PWRDN: cluster-gdhs {
> > > +				compatible = "domain-idle-state";
> > > +				arm,psci-suspend-param = <0x1000030>;
> > > +				entry-latency-us = <2000>;
> > > +				exit-latency-us = <2000>;
> > > +				min-residency-us = <6000>;
> > > +			};
> > >  		};
> > >  	};
> >
> > I was trying to understand the composition of composite state parameters
> > in this series and that made me look at these DT examples.
> >
> This was meant to depict a hierarchical state format for OSI.
>

Hmm, I am more confused. We have 2 formats: original and extended.

1. Original:
	31:26 Reserved. Must be zero.
	25:24 PowerLevel
	23:17 Reserved. Must be zero.
	16 StateType
	15:0 StateID
2. Extended
	31 Reserved. Must be zero.
	30 StateType
	29:28 Reserved. Must be zero.
	27:0 StateID

I was trying to match them to that. I think I commented on other patches.
I think simple OR logic breaks with extended format easily if StateIDs
are not carefully crafted which is not mandated and hence the trouble.
The same holds to original format but with PowerLevel, it slightly
relaxing things a bit but still it needs to be crafted when firmware
decides these parameters. E.g.: what is done with HiKey platform is
completely wrong.

It's helpful if we want to avoid save/restore for retention states.
CPU_PM_CPU_IDLE_ENTER_RETENTION vs CPU_PM_CPU_IDLE_ENTER

> > What format does the above platform use ? I tried matching them to
> > both original as well as extended format and I fail to understand.
> > Assuming original format:
> > 	State         power_state PowerLevel  StateType     StateID
> > 	SPC           0x40000002   0(core)    0(Retention)  0x2 (Res0 b[29]=1?)
> > 	CLUSTER_RET   0x1000010   1(clusters) 0(Retention)  0x10
> > 	CLUSTER_PWRDN 0x1000030   1(clusters) 0(Retention?) 0x30
> > Now extended format:
> > 	State         power_state StateType     StateID
> > 	SPC           0x40000002  0(Retention)  0x40000002 (Res0 b[29]=1?)
> > 	CLUSTER_RET   0x1000010   0(Retention)  0x1000010
> The composite state would comprise of CPU state and  Cluster state.
> So for the last CPU entering idle -
> (CLUSTER_RET | SPC)
> 0x41000012
> > 	CLUSTER_PWRDN 0x1000030   0(Retention?) 0x1000030
> >
> (CLUSTER_PWRDN | SPC)
> 0x41000032
>
> Hope this helps.
>

I just follow OR logic. I have made wrong reference to bit 29 above(I
can't read simple 32 bit number anymore :(), it should bit 30 and if
this platform follow extended state, then it makes some sense. But
I expect CLUSTER_PWRDN also to have bit 30 set. I tried to match to both
formats and failed to understand which it follows, so thought of asking.

--
Regards,
Sudeep

  reply index

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-13 19:22 [PATCH 00/18] ARM/ARM64: Support hierarchical CPU arrangement for PSCI Ulf Hansson
2019-05-13 19:22 ` [PATCH 01/18] dt: psci: Update DT bindings to support hierarchical PSCI states Ulf Hansson
2019-07-19 11:29   ` Lorenzo Pieralisi
2019-05-13 19:22 ` [PATCH 02/18] of: base: Add of_get_cpu_state_node() to get idle states for a CPU node Ulf Hansson
2019-05-13 19:22 ` [PATCH 03/18] cpuidle: dt: Support hierarchical CPU idle states Ulf Hansson
2019-05-13 19:22 ` [PATCH 04/18] ARM/ARM64: cpuidle: Let back-end init ops take the driver as input Ulf Hansson
2019-06-07 15:00   ` Sudeep Holla
2019-06-10 10:20     ` Ulf Hansson
2019-05-13 19:22 ` [PATCH 05/18] drivers: firmware: psci: Simplify state node parsing Ulf Hansson
2019-06-07 15:01   ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 06/18] drivers: firmware: psci: Support hierarchical CPU idle states Ulf Hansson
2019-06-07 15:03   ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 07/18] drivers: firmware: psci: Prepare to use OS initiated suspend mode Ulf Hansson
2019-06-07 15:17   ` Sudeep Holla
2019-06-10 10:21     ` Ulf Hansson
2019-06-10 10:42       ` Sudeep Holla
2019-07-16 14:53   ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 08/18] drivers: firmware: psci: Prepare to support PM domains Ulf Hansson
2019-06-07 15:21   ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 09/18] drivers: firmware: psci: Add support for PM domains using genpd Ulf Hansson
2019-06-07 15:27   ` Sudeep Holla
2019-06-10 10:21     ` Ulf Hansson
2019-06-10 10:59       ` Sudeep Holla
2019-07-16 15:05   ` Sudeep Holla
2019-07-18 11:04     ` Ulf Hansson
2019-07-18 13:19       ` Sudeep Holla
2019-07-18 17:57         ` Lina Iyer
2019-07-19  9:45           ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 10/18] drivers: firmware: psci: Add hierarchical domain idle states converter Ulf Hansson
2019-07-09 15:31   ` Lorenzo Pieralisi
2019-07-16  8:45     ` Ulf Hansson
2019-07-16 14:51       ` Lorenzo Pieralisi
2019-07-18 11:43         ` Ulf Hansson
2019-07-18 13:36           ` Lorenzo Pieralisi
2019-05-13 19:22 ` [PATCH 11/18] drivers: firmware: psci: Introduce psci_dt_topology_init() Ulf Hansson
2019-05-13 19:22 ` [PATCH 12/18] drivers: firmware: psci: Add a helper to attach a CPU to its PM domain Ulf Hansson
2019-05-13 19:22 ` [PATCH 13/18] drivers: firmware: psci: Attach the CPU's device " Ulf Hansson
2019-05-13 19:22 ` [PATCH 14/18] drivers: firmware: psci: Manage runtime PM in the idle path for CPUs Ulf Hansson
2019-07-16 15:53   ` Lorenzo Pieralisi
2019-07-18 10:35     ` Ulf Hansson
2019-07-18 13:30       ` Lorenzo Pieralisi
2019-07-18 16:54         ` Ulf Hansson
2019-07-18 17:41           ` Lina Iyer
2019-07-18 21:49             ` Ulf Hansson
2019-07-19 10:02               ` Lorenzo Pieralisi
2019-05-13 19:22 ` [PATCH 15/18] drivers: firmware: psci: Support CPU hotplug for the hierarchical model Ulf Hansson
2019-06-07 15:31   ` Sudeep Holla
2019-06-10 10:21     ` Ulf Hansson
2019-06-10 11:02       ` Sudeep Holla
2019-05-13 19:22 ` [PATCH 16/18] arm64: kernel: Respect the hierarchical CPU topology in DT for PSCI Ulf Hansson
2019-05-13 19:22 ` [PATCH 17/18] arm64: dts: Convert to the hierarchical CPU topology layout for MSM8916 Ulf Hansson
2019-07-16 14:47   ` Sudeep Holla
2019-07-16 20:36     ` Lina Iyer
2019-07-17 17:18       ` Sudeep Holla [this message]
2019-05-13 19:23 ` [PATCH 18/18] arm64: dts: hikey: Convert to the hierarchical CPU topology layout Ulf Hansson
2019-07-16 14:47   ` Sudeep Holla
2019-07-18 10:48     ` Ulf Hansson
2019-07-18 13:11       ` Sudeep Holla
2019-05-14  8:08 ` [PATCH 00/18] ARM/ARM64: Support hierarchical CPU arrangement for PSCI Rafael J. Wysocki
2019-05-14  8:58   ` Ulf Hansson
2019-06-07 15:42     ` Sudeep Holla
2019-06-07 19:34       ` Bjorn Andersson
2019-06-10 10:32         ` Sudeep Holla
2019-06-10 15:54           ` Ulf Hansson
2019-06-10 17:16             ` Lorenzo Pieralisi
2019-06-10 18:57               ` Ulf Hansson
2019-06-18 11:56                 ` Ulf Hansson
2019-06-07 11:19 ` Ulf Hansson

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=20190717171856.GF18370@e107155-lin \
    --to=sudeep.holla@arm.com \
    --cc=Lorenzo.Pieralisi@arm.com \
    --cc=amit.kucheria@linaro.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=geert+renesas@glider.be \
    --cc=ilina@codeaurora.org \
    --cc=khilman@kernel.org \
    --cc=lina.iyer@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --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=niklas.cassel@linaro.org \
    --cc=rjw@rjwysocki.net \
    --cc=rplsssn@codeaurora.org \
    --cc=sboyd@kernel.org \
    --cc=souvik.chakravarty@arm.com \
    --cc=tony@atomide.com \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.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-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/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-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

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


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