Date:	Wed, 11 Jun 2014 19:04:58 +0530
From:	Abhilash Kesavan <kesavan.abhilash@...il.com>
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>,
	Chander Kashyap <chander.kashyap@...aro.org>,
	"olof@...om.net" <olof@...om.net>,
	Tushar Behera <trblinux@...il.com>,
	Thomas Abraham <thomas.ab@...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

Hi Lorenzo and Chander,

On Wed, Jun 11, 2014 at 6:45 PM, 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. */"
This is something that pends on me.
>> >
>> > 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 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.
> 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.

I have already implemented the missing cluster power down code and
tested it on an exynos5420 based chromebook. I plan to post the patch
as soon as I am back in office, which should be early next week.

Regards,
Abhilash
>
> 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
>>
>
> --
> 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-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