[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADWw7OiD8TdN75G0zecowAL9GniSU9hKLN0_H6UQL85LX1E9Xw@mail.gmail.com>
Date: Wed, 5 Nov 2014 15:31:02 +0530
From: Chander Kashyap <k.chander@...sung.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc: Chander Kashyap <k.chander@...sung.com>,
Mark Rutland <Mark.Rutland@....com>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
Catalin Marinas <Catalin.Marinas@....com>,
"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
"rjw@...ysocki.net" <rjw@...ysocki.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kgene.kim@...sung.com" <kgene.kim@...sung.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] arm64: dts: exynos7: add support for cpuidle core power down
Sorry for very late response. As i was on vacation so couldn’t reply.
On Tue, Oct 21, 2014 at 10:03 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@....com> wrote:
> On Fri, Oct 17, 2014 at 10:43:59AM +0100, Chander Kashyap wrote:
>> Hi Lorenzo,
>>
>> On Wed, Oct 15, 2014 at 2:30 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@....com> wrote:
>> > On Wed, Oct 15, 2014 at 07:35:20AM +0100, Chander Kashyap wrote:
>> >> Exynos7 has core power down state where cores can be powered off independently.
>> >> This patch adds support for this state.
>> >
>> > Please tell us more about the idle-state values you are adding, in particular
>> > entry, exit latencies and min-residency values.
>>
>> Entry latency: This value is calculated as follows:
>>
>> On entry to arm64_enter_idle_state:
>> timestamp1 = ktimeget();
>>
>> after returning from cpu_suspend()
>>
>> timestamp2 = ktimeget();
>>
>> latency = timestamp2-timestamp1;
>>
>> Cpu is not allowed to enter core powerdown by skipping wfi instruction at end.
> This may not be the worst case (because the worst case depends on the state
> of the cache in the core unless the latency is power down command dominated,
> so at the cost of being pedantic, please make sure that's what you are
> measuring and document it in the commit log).
>
If i understood correctly you are referring to cache flush time.
The measured entry latency time is averaged time for 100000
transactions with varying load.
I will document entry latency calculation in the commit message.
>> Hence calculated time contains entry time + failure exit time.
>>
>>
>> Regarding
>> exit-latency and target-residency time, got these values from HW team.
>>
>> I am using these as initial values and I will be working on optimizing
>> these values with further experiments.
>> If you could suggest any formal method of deriving these values, i can
>> try those methods as well.
>
> Well, you have to set the core/cluster in worst case scenario and
> compute the break-even residency against wfi (since you have two
> states); it certainly has a dependency on PSCI implementation too among
> other things.
>
> exit-latency should come from HW design even though there is a cache
> refill factor to be considered too and should be factored in.
Exit and target residency are provided by HW team.
I will post the V3 with changed commit message.
>
> Lorenzo
>
>>
>> >
>> >> Signed-off-by: Chander Kashyap <k.chander@...sung.com>
>> >> ---
>> >> This patch has following dependencies:
>> >> - [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
>> >> http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
>> >> - [PATCH v9 0/8] ARM generic idle states
>> >> http://permalink.gmane.org/gmane.linux.power-management.general/49224
>> >
>> > Series above was merged, so dependency is stale.
>>
>> i will remove this
>>
>> >
>> >> arch/arm64/boot/dts/exynos/exynos7.dtsi | 18 ++++++++++++++++++
>> >> 1 file changed, 18 insertions(+)dont
>> >>
>> >> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> index ce221ac..8e0a034 100644
>> >> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> @@ -36,6 +36,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >
>> > I would add cpu-idle-states phandle after the reg property, as defined
>> > in the idle states bindings.
>>
>> i will move this after reg property.
>>
>> >
>> >> reg = <0x0>;
>> >> };
>> >>
>> >> @@ -43,6 +44,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x1>;
>> >> };
>> >>
>> >> @@ -50,6 +52,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x2>;
>> >> };
>> >>
>> >> @@ -57,8 +60,23 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x3>;
>> >> };
>> >> +
>> >> + idle-states {
>> >> + entry-method = "arm,psci";
>> >> +
>> >> + CPU_SLEEP: cpu-sleep {
>> >> + compatible = "arm,idle-state";
>> >> + local-timer-stop;
>> >> + arm,psci-suspend-param = <0x0010000>;
>> >> + entry-latency-us = <20>;
>> >> + exit-latency-us = <150>;
>> >> + min-residency-us = <2100>;
>> >> + status = "enabled";
>> >
>> > status ? This is not a documented property. If you need it please explain
>> > why, define its bindings and we can see how to accommodate it.
>>
>> I will add okay for status property. As per the bindings posted by you.
>>
>> regards,
>> >
>> > Thank you,
>> > Lorenzo
>> >
>> >> + };
>> >> + };
>> >> };
>> >>
>> >> psci {
>> >> --
>> >> 1.7.9.5
>> >>
>> >>
>> >
>> >
>> > _______________________________________________
>> > linux-arm-kernel mailing list
>> > linux-arm-kernel@...ts.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists