Hello Bartlomiej and Lorenzo, Thanks a lot for your explanations. On 08/27/2015 06:58 PM, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Tuesday, August 25, 2015 05:09:32 PM Lorenzo Pieralisi wrote: >> On Tue, Aug 25, 2015 at 03:35:29PM +0100, Bartlomiej Zolnierkiewicz wrote: >>> >>> [ added Lorenzo and linux-pm to Cc: ] >>> >>> Hi, >>> >>> On Tuesday, August 25, 2015 11:43:38 AM Javier Martinez Canillas wrote: >>>> [adding Kevin Hilman as cc who was also interested in CPUidle for Exynos] >>>> >>>> Hello Krzysztof, >>>> >>>> On 08/23/2015 03:26 AM, Krzysztof Kozlowski wrote: >>>> >>>> [snip] >>>> >>>>> 2015-08-21 16:21 GMT+09:00 Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>: >>>>> >>>>> The big.LITTLE cpuidle driver is not a typical Exynos cpuidle driver. >>>>> It only executes CPU suspend on a cluster which essentially is a power >>>>> down operation. >>>>> >>>> >>>> You are correct, looking at the the big.LITTLE CPUidle driver I see that >>>> it only has two C-states: C0 (normal WFI) and C1 (single CPU power-down) >>>> which as you said, places the CPU into power-down mode by using the MCPM >>>> infrastructure so it's basically a CPU suspend AFAIU. >>>> >>>> So what you are saying is that there are deeper C-states supported by the >>>> Exynos 542x SoC family but these are not handled by the b.L CPUidle driver. >>>> >>>>> When we talk about cpuidle on Exynos, we have in mind one of sleep >>>>> modes: AFTR or LPA (sometimes instead of LPA there is LPD or W-AFTR). >>>>> Actually this is more like a system idle mode, not CPU idle. The power >>>>> savings are much bigger than disabling only one cluster. >>>>> >>>> >>>> Interesting, I was not aware of AFTR and LPA but I looked in the manual now. >>>> Thanks a lot for the information. >>>> >>>> I see that the Exynos CPUidle driver (drivers/cpuidle/cpuidle-exynos.c) also >>>> has only two C-states (WFI and C1) but C1 makes the system to enter in AFTR >>>> (system-level power gating). >>>> >>>> This is similar to what the downstream ChromiumOS 3.8 kernel CPUidle driver >>>> does IIUC [0]. >>> >>> Yes but upstream does it in a clean way, has support for platforms >>> requiring secure firmware operations and also implements coupled >>> AFTR mode on a few platforms. >>> >>>>> So the question is still valid - whether someone wants or plans to >>>>> implement cpuidle for Exynos 542x family. Odroid XU3 is not a priority >>>>> here because energy consumption is not an issue there. This is not a >>>>> mobile device. >>>>> >>>> >>>> That's true but it will be interesting for the 5420 and 5800 based >>>> Chromebooks since optimizing power consumption would be useful there. >>> >>> I would be happy to help with reviewing patches etc. but personally >>> I don't have any plans for doing this work. I may look into adding >>> support for newer ARM64 SoCs (Exynos5433) if I find some extra time >>> (quite unlikely currently). >>> >>>> I thought that big.LITTLE platforms were encouraged to use the generic b.L >>>> CPUidle driver just like DT platforms should use the generic CPUFreq DT >>>> driver but I guess I misunderstood. >>>> >>>> So the b.L CPUidle driver is only to have minimum CPUidle support but a SoC >>>> specific driver is needed to fine tune and get most out of the platform? >>>> >>>> Or should the b.L CPUidle driver be extended to add per platform C-states? >>> >>> This is a good question. Daniel/Lorenzo? >> >> To move the b.L driver to multiple C-states we should first convert it to >> the generic CPUidle driver (by defining an MCPM enable-method): >> >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/cpuidle-arm.c?id=refs/tags/v4.2-rc8 >> >> Then we have to figure out how to determine how many CPUidle drivers we >> have to create (since idle states are different on different CPUs), since >> using the MIDR does not really scale. >> >> For certain I won't support coupled C-states in the DT idle states code, >> and every platform requiring them is considered buggy and not worth >> merging in the mainline kernel from now onwards, HW should be fixed, >> eventually, I am not willing to see code like >> drivers/cpuidle/cpuidle-exynos.c in the mainline kernel anymore, >> I am sorry. > > For Exynos chipsets coupled C-states are not strictly required for > having cpuidle support but make it more useful. On Exynos chipsets > secondary CPUs must be disabled to allow system to go into deeper > idle modes and this is assured by using coupled C-states. Original > Android drivers don't use coupled C-states and depend on the rest > of the system to offline secondary CPUs when not needed. > > I would of course prefer not to have handle this in software and > be automatically handled by hardware/firmware but that doesn't mean > that we should be declining mainline support for "ugly" hardware > (this has never been "the Linux way" of doing things). > > Coming back to the platforms in question (ARM32 big.LITTLE based > Odroid XU3 boards etc.) they have been shipped long time ago and > the best we can do nowadays is to support them as well as possible. > I agree. > If somebody wants to implement a separate Exynos542x/Exynos5800 > big.LITTLE cpuidle driver for them I see no problem with it and I'm > willing to help in maintaining it. > Ok, I'll see if I can take a look what is needed to implement a Exynos542x CPUidle driver. I'm quite busy with other stuff right now but I should be less busy in a couple of weeks. Maybe is a little bit out of topic but since Anand also asked about CPUFreq support, are you planning on re-posting your "cpufreq: add generic cpufreq driver support for Exynos5250/5800 platforms" [0] series? >> I think it is time to draw a line with CPUidle on ARM, take stock, clean >> it up and find a way forward, current situation is a potpourri of solutions >> that all differ in a slightly incompatible way. >> >> Support on ARM64 SoC must be PSCI based and for that there is already >> support in the mainline kernel (through the PSCI back-end and the DT >> idle states bindings). > > I hope that on ARM64 PSCI will be used. However I have no access to > chipset designers / original firmware authors so I can't tell for > sure. > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > [0]: https://lkml.org/lkml/2015/4/21/520 Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html