From: Lukasz Luba <lukasz.luba@arm.com> To: Marek Szyprowski <m.szyprowski@samsung.com>, Anand Moon <linux.amoon@gmail.com> Cc: Linux PM list <linux-pm@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, linux-samsung-soc@vger.kernel.org, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, Krzysztof Kozlowski <krzk@kernel.org> Subject: Re: [PATCH 0/4] Restore big.LITTLE cpuidle driver for Exynos Date: Wed, 17 Jun 2020 11:40:42 +0100 [thread overview] Message-ID: <6b2ac503-62e1-35a6-6dc7-232597367396@arm.com> (raw) In-Reply-To: <3e6b5dbb-a8a2-e3db-d740-53e13676455c@samsung.com> On 6/17/20 10:48 AM, Marek Szyprowski wrote: > Hi Anand, > > On 16.06.2020 22:58, Anand Moon wrote: >> On Tue, 16 Jun 2020 at 13:44, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >>> The ARM big.LITTLE cpuidle driver has been enabled and tested on Samsung >>> Exynos 5420/5800 based Peach Pit/Pi Chromebooks and in fact it worked >>> only on those boards. >>> >>> However, support for it was broken by the commit 833b5794e330 ("ARM: >>> EXYNOS: reset Little cores when cpu is up") and then never enabled in the >>> exynos_defconfig. This patchset provides the needed fix to the common >>> code and restores support for it. Thanks to Lukasz Luba who motivated me >>> to take a look into this issue. >>> >> Thanks for this updates. >> >> But I feel some DTS changes are missing for example >> d2e5c871ed8a drivers: cpuidle: initialize big.LITTLE driver through DT > > This is not strictly needed. The bl-cpuidle matches also to the A7/A15 > CPU product ids and it is properly instantiated on the Peach Pit/Pi > Chromebooks. Those CPU DT properties were added as a future-proof > generic solution. I won't hurt to add them though. > >> But I feel that this feature is not working as desired since >> still some missing code changes for cluster idle states are missing. >> like clock PWR_CTR and PWR_CTRL2. > > I cannot judge now. All I can test now is a that the boards enters those > idle states and system works stable. I cannot measure power consumption, > because currently I have only remote access to the boards. I agree with Marek. This can be done incrementally. The series fixes the code path which was working. After the investigation with a power meter, a proper set of new patches might come if needed. As a hint to measure this power consumption difference, because it might be tricky, I would suggest to heat up the SoC. The main difference between wfi and deeper idle which cut the power to some components (like caches) should be seen at higher voltage OPP and higher temperature. It's due to the fact that static power (leakage) is related to Vdd and temperature - higher voltage -> higher leakage higher temp -> higher leakage This difference (idle state 0 vs 1) should be amplified in the above scenario and easier to measure. I am going to review this series after finishing hotplug tests. Regards, Lukasz
WARNING: multiple messages have this Message-ID (diff)
From: Lukasz Luba <lukasz.luba@arm.com> To: Marek Szyprowski <m.szyprowski@samsung.com>, Anand Moon <linux.amoon@gmail.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, Linux PM list <linux-pm@vger.kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, Krzysztof Kozlowski <krzk@kernel.org>, linux-samsung-soc@vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 0/4] Restore big.LITTLE cpuidle driver for Exynos Date: Wed, 17 Jun 2020 11:40:42 +0100 [thread overview] Message-ID: <6b2ac503-62e1-35a6-6dc7-232597367396@arm.com> (raw) In-Reply-To: <3e6b5dbb-a8a2-e3db-d740-53e13676455c@samsung.com> On 6/17/20 10:48 AM, Marek Szyprowski wrote: > Hi Anand, > > On 16.06.2020 22:58, Anand Moon wrote: >> On Tue, 16 Jun 2020 at 13:44, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >>> The ARM big.LITTLE cpuidle driver has been enabled and tested on Samsung >>> Exynos 5420/5800 based Peach Pit/Pi Chromebooks and in fact it worked >>> only on those boards. >>> >>> However, support for it was broken by the commit 833b5794e330 ("ARM: >>> EXYNOS: reset Little cores when cpu is up") and then never enabled in the >>> exynos_defconfig. This patchset provides the needed fix to the common >>> code and restores support for it. Thanks to Lukasz Luba who motivated me >>> to take a look into this issue. >>> >> Thanks for this updates. >> >> But I feel some DTS changes are missing for example >> d2e5c871ed8a drivers: cpuidle: initialize big.LITTLE driver through DT > > This is not strictly needed. The bl-cpuidle matches also to the A7/A15 > CPU product ids and it is properly instantiated on the Peach Pit/Pi > Chromebooks. Those CPU DT properties were added as a future-proof > generic solution. I won't hurt to add them though. > >> But I feel that this feature is not working as desired since >> still some missing code changes for cluster idle states are missing. >> like clock PWR_CTR and PWR_CTRL2. > > I cannot judge now. All I can test now is a that the boards enters those > idle states and system works stable. I cannot measure power consumption, > because currently I have only remote access to the boards. I agree with Marek. This can be done incrementally. The series fixes the code path which was working. After the investigation with a power meter, a proper set of new patches might come if needed. As a hint to measure this power consumption difference, because it might be tricky, I would suggest to heat up the SoC. The main difference between wfi and deeper idle which cut the power to some components (like caches) should be seen at higher voltage OPP and higher temperature. It's due to the fact that static power (leakage) is related to Vdd and temperature - higher voltage -> higher leakage higher temp -> higher leakage This difference (idle state 0 vs 1) should be amplified in the above scenario and easier to measure. I am going to review this series after finishing hotplug tests. Regards, Lukasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-06-17 10:40 UTC|newest] Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20200616081248eucas1p168faa343ce333a28c8fd3cf9a6a58b3c@eucas1p1.samsung.com> 2020-06-16 8:12 ` [PATCH 0/4] Restore big.LITTLE cpuidle driver for Exynos Marek Szyprowski 2020-06-16 8:12 ` Marek Szyprowski [not found] ` <CGME20200616081249eucas1p151a8892ca0abfa3108955e1fc5054fc3@eucas1p1.samsung.com> 2020-06-16 8:12 ` [PATCH 1/4] ARM: exynos: Apply little core workaround only under secure firmware Marek Szyprowski 2020-06-16 8:12 ` Marek Szyprowski 2020-06-17 16:26 ` Lukasz Luba 2020-06-17 16:26 ` Lukasz Luba 2020-06-22 17:19 ` Krzysztof Kozlowski 2020-06-29 8:54 ` Marek Szyprowski 2020-06-29 8:54 ` Marek Szyprowski 2020-06-29 9:13 ` Krzysztof Kozlowski 2020-06-29 9:13 ` Krzysztof Kozlowski [not found] ` <CGME20200629100230eucas1p1bf07ca4c84ba6be1fbdd80c45d077518@eucas1p1.samsung.com> 2020-06-29 10:02 ` [PATCH v2 1/4] ARM: exynos: MCPM: Restore big.LITTLE cpuidle support Marek Szyprowski 2020-06-29 10:02 ` Marek Szyprowski 2020-06-29 11:05 ` Lukasz Luba 2020-06-29 11:05 ` Lukasz Luba 2020-06-29 18:28 ` Krzysztof Kozlowski 2020-06-29 18:28 ` Krzysztof Kozlowski [not found] ` <CGME20200616081249eucas1p2ad165b545f265bec88a06270d55fce76@eucas1p2.samsung.com> 2020-06-16 8:12 ` [PATCH 2/4] cpuidle: big.LITTLE: enable driver only on Peach-Pit/Pi Chromebooks Marek Szyprowski 2020-06-16 8:12 ` Marek Szyprowski 2020-06-22 17:19 ` Krzysztof Kozlowski 2020-06-24 10:05 ` Bartlomiej Zolnierkiewicz 2020-06-24 10:05 ` Bartlomiej Zolnierkiewicz 2020-08-17 15:39 ` Krzysztof Kozlowski 2020-08-17 15:39 ` Krzysztof Kozlowski 2020-08-24 8:15 ` Daniel Lezcano 2020-08-24 8:15 ` Daniel Lezcano 2020-08-24 8:26 ` Krzysztof Kozlowski 2020-08-24 8:26 ` Krzysztof Kozlowski 2020-08-24 9:14 ` Daniel Lezcano 2020-08-24 9:14 ` Daniel Lezcano [not found] ` <CGME20200616081250eucas1p2de7110898dad050866d78cb5d5120422@eucas1p2.samsung.com> 2020-06-16 8:12 ` [PATCH 3/4] ARM: exynos_defconfig: Enable big.LITTLE cpuidle driver Marek Szyprowski 2020-06-16 8:12 ` Marek Szyprowski 2020-06-22 17:20 ` Krzysztof Kozlowski 2020-06-29 11:35 ` Marek Szyprowski 2020-06-29 11:35 ` Marek Szyprowski 2020-06-24 10:06 ` Bartlomiej Zolnierkiewicz 2020-06-24 10:06 ` Bartlomiej Zolnierkiewicz [not found] ` <CGME20200616081250eucas1p2a98f8810962ddc692fa5588a74f911b3@eucas1p2.samsung.com> 2020-06-16 8:12 ` [PATCH 4/4] ARM: multi_v7_defconfig: " Marek Szyprowski 2020-06-16 8:12 ` Marek Szyprowski 2020-06-24 10:06 ` Bartlomiej Zolnierkiewicz 2020-06-24 10:06 ` Bartlomiej Zolnierkiewicz 2020-06-16 20:58 ` [PATCH 0/4] Restore big.LITTLE cpuidle driver for Exynos Anand Moon 2020-06-16 20:58 ` Anand Moon 2020-06-17 9:48 ` Marek Szyprowski 2020-06-17 9:48 ` Marek Szyprowski 2020-06-17 10:40 ` Lukasz Luba [this message] 2020-06-17 10:40 ` Lukasz Luba 2020-06-17 10:57 ` Anand Moon 2020-06-17 10:57 ` Anand Moon
Reply instructions: You may reply publicly 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=6b2ac503-62e1-35a6-6dc7-232597367396@arm.com \ --to=lukasz.luba@arm.com \ --cc=b.zolnierkie@samsung.com \ --cc=daniel.lezcano@linaro.org \ --cc=krzk@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux.amoon@gmail.com \ --cc=lorenzo.pieralisi@arm.com \ --cc=m.szyprowski@samsung.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.