Message-ID: <CANuQgHGM=NuyBvJfGRuDk3MepzBzqC7du6rsK1GmK-Of0c5-vA@mail.gmail.com>
Date:	Wed, 11 Jun 2014 18:59:46 +0530
From:	Chander Kashyap <chander.kashyap@...aro.org>
To:	Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:	Chander Kashyap <k.chander@...sung.com>,
	Nicolas Pitre <nicolas.pitre@...aro.org>,
	Doug Anderson <dianders@...omium.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Kukjin Kim <kgene.kim@...sung.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	Kevin Hilman <khilman@...aro.org>,
	Andrew Bresticker <abrestic@...omium.org>,
	Inderpal Singh <inderpal.s@...sung.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"olof@...om.net" <olof@...om.net>,
	Tushar Behera <trblinux@...il.com>,
	Thomas Abraham <thomas.ab@...sung.com>,
	Abhilash Kesavan <a.kesavan@...sung.com>,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start

On 11 June 2014 18:45, Lorenzo Pieralisi <lorenzo.pieralisi@....com> wrote:
> On Wed, Jun 11, 2014 at 01:14:21PM +0100, Chander Kashyap wrote:
>> On Wed, Jun 11, 2014 at 3:43 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@....com> wrote:
>> > On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote:
>> >> Hi Doug,
>> >>
>> >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@...aro.org> wrote:
>> >> > On Tue, 10 Jun 2014, Doug Anderson wrote:
>> >> >
>> >> >> My S-state knowledge is not strong, but I believe that Lorenzo's
>> >> >> questions matter if we're using S2 for CPUidle (where we actually turn
>> >> >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle
>> >> >> (where we just enter WFI/WFE).
>> >> >>
>> >>
>> >> No Its not plain WFI.
>> >>
>> >> All cores in Exynos5420 can be powered off independently.
>> >> This functionality has been tested.
>> >>
>> >> Below is the link for the posted patches.
>> >>
>> >> https://lkml.org/lkml/2014/6/10/194
>> >>
>> >> And as Nicolas wrote, these patches need MCPM for that.
>> >
>> > Chander, I cast a look into the code and I have a question
>> > (you also told me on CPUidle review that only core off
>> > is supported in CPUidle).
>> >
>> > When you say all cores can be powered off independently, do
>> > you also mean that clusters can be powered off (in CPUidle) ?
>> >
>> > I am pointing this out since in the MCPM backend I noticed:
>> >
>> > "/* TODO: Turn off the cluster here to save power. */"
>> >
>> > I do not see any cluster power down request in the down path.
>> >
>> > If I am wrong, ignore my message, I am just writing to help.
>> >
>> > If you can only power down cores, but not the clusters on idle,
>> > please keep in mind that you are currently cleaning and invalidating
>> > the L2 when last man is running and this must not be taken
>> > lightly since, if L2 stays on, that's a massive energy waste
>> > for nothing.
>> >
>> > So, if clusters stay up, you _have_ to tweak the MCPM backend slightly
>> > to avoid cleaning L2, that's pivotal.
>>
>> Hi Lorenzo,
>> Cluster shutdown is in progress. Abhilash will add support for that.
>>
>> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31104.html
>

Hi Lorenzo,

> Hi Chander,
>
> thanks. So, as a heads-up:
>
> 1) if you merge CPUidle support now, as it is it is at least suboptimal, may
>    even burn more energy than it saves. Latencies in the bL idle driver
>    are likely to be wrong, since they are for cluster shutdown and for
>    TC2, not core power gating that should require shorter target_residencies.
>    On top of that, L2 is cleaned and invalidated for nothing.

Yes true.

> 2) when cluster support is merged, you might want to extend the CPUidle
>    driver to add an additional state (ie C1 core gating, C2 cluster
>    gating) and to do that you should extend the driver and the MCPM
>    back-end accordingly, I discussed that with Nico already some time ago
>    and actually it should be fairly easy to do but we have got to talk
>    about that.
>

Yes thats the final goal to add two states (C1 core gating, C2 cluster gating).
But that will be done in combination with yours patches(to pass
cpuidle data thru DT).

So finally the mcpm backend will take care for c1 and c2 cpuidle state
implementation.

> Thank you,
> Lorenzo
>
>>
>>
>> >
>> > Lorenzo
>> >
>> >>
>> >> >> I believe that in ChromeOS we use S1 CPUidle and that it works fine.
>> >> >> We've never implemented S2 that I'm aware of.
>> >> >
>> >> > You'll have to rely on MCPM for that.  That's probably why it hasn't
>> >> > been implemented before.
>> >> >
>> >> >
>> >> > Nicolas
>> >> >
>> >> > _______________________________________________
>> >> > 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/
>> >>
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> > the body of a message to majordomo@...r.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>



-- 
with warm regards,
Chander Kashyap
--
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